Skip to content

ENG-1634 Re-sync nodes if needed at the time the users publish the node #951

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

Merged
maparent merged 9 commits into from
Apr 14, 2026
+121 −68
Merged

ENG-1634 Re-sync nodes if needed at the time the users publish the node #951

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d8578b2
Do a sync on publish
maparent Apr 10, 2026
3fec3af
break recursion
maparent Apr 10, 2026
801c1f4
isolate asset syncing, to avoid syncing twice
maparent Apr 12, 2026
6f9b189
nit: remove node ref in commented code
maparent Apr 12, 2026
9ca0b9c
lint
maparent Apr 13, 2026
4027e1f
devin suggestion: fail on a per-node basis
maparent Apr 13, 2026
f9b90f3
eliminate republish flag
maparent Apr 14, 2026
660ccb3
Notify user of sync failure
maparent Apr 14, 2026
c1440af
forgot a return value type
maparent Apr 14, 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
77 changes: 14 additions & 63 deletions apps/obsidian/src/utils/publishNode.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import type { FrontMatterCache, TFile } from "obsidian";
import { Notice } from "obsidian";
import type { default as DiscourseGraphPlugin } from "~/index";
import { getLoggedInClient, getSupabaseContext } from "./supabaseContext";
import { addFile } from "@repo/database/lib/files";
import mime from "mime-types";
import type { DGSupabaseClient } from "@repo/database/lib/client";
import {
getRelationsForNodeInstanceId,
Expand All @@ -15,7 +12,10 @@ import {
} from "./relationsStore";
import type { RelationInstance } from "~/types";
import { getAvailableGroupIds } from "./importNodes";
import { syncAllNodesAndRelations } from "./syncDgNodesToSupabase";
import {
syncAllNodesAndRelations,
syncPublishedNodeAssets,
} from "./syncDgNodesToSupabase";
import type { DiscourseNodeInVault } from "./getDiscourseNodes";
import type { SupabaseContext } from "./supabaseContext";
import type { TablesInsert } from "@repo/database/dbTypes";
Expand Down Expand Up @@ -262,6 +262,8 @@ export const publishNode = async ({
if (myGroups.size === 0) throw new Error("Cannot get group");
const existingPublish =
(frontmatter.publishedToGroups as undefined | string[]) || [];
// Hopefully temporary workaround for sync bug
await syncAllNodesAndRelations(plugin);
const commonGroups = existingPublish.filter((g) => myGroups.has(g));
// temporary single-group assumption
const myGroup = (commonGroups.length > 0 ? commonGroups : [...myGroups])[0]!;
Expand Down Expand Up @@ -506,65 +508,14 @@ export const publishNodeToGroup = async ({
});
}
}

// Always sync non-text assets when node is published to this group
const existingFiles: string[] = [];
const existingReferencesReq = await client
.from("my_file_references")
.select("*")
.eq("space_id", spaceId)
.eq("source_local_id", nodeId);
if (existingReferencesReq.error) {
console.error(existingReferencesReq.error);
return;
}
const existingReferencesByPath = Object.fromEntries(
existingReferencesReq.data.map((ref) => [ref.filepath, ref]),
);

for (const attachment of attachments) {
const mimetype = mime.lookup(attachment.path) || "application/octet-stream";
if (mimetype.startsWith("text/")) continue;
// Do not use standard upload for large files
if (attachment.stat.size >= 6 * 1024 * 1024) {
new Notice(
`Asset file ${attachment.path} is larger than 6Mb and will not be uploaded`,
);
continue;
}
existingFiles.push(attachment.path);
const existingRef = existingReferencesByPath[attachment.path];
if (
!existingRef ||
new Date(existingRef.last_modified + "Z").valueOf() <
attachment.stat.mtime
) {
const content = await plugin.app.vault.readBinary(attachment);
await addFile({
client,
spaceId,
sourceLocalId: nodeId,
fname: attachment.path,
mimetype,
created: new Date(attachment.stat.ctime),
lastModified: new Date(attachment.stat.mtime),
content,
});
}
}
let cleanupCommand = client
.from("FileReference")
.delete()
.eq("space_id", spaceId)
.eq("source_local_id", nodeId);
if (existingFiles.length)
cleanupCommand = cleanupCommand.notIn("filepath", [
...new Set(existingFiles),
]);
const cleanupResult = await cleanupCommand;
// do not fail on cleanup
if (cleanupResult.error) console.error(cleanupResult.error);

await syncPublishedNodeAssets({
plugin,
client,
nodeId,
spaceId,
file,
attachments,
});
if (!existingPublish.includes(myGroup))
await plugin.app.fileManager.processFrontMatter(
file,
Expand Down
112 changes: 107 additions & 5 deletions apps/obsidian/src/utils/syncDgNodesToSupabase.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
/* eslint-disable @typescript-eslint/naming-convention */
import { FrontMatterCache, Notice, TFile } from "obsidian";
import { Notice, TFile } from "obsidian";
import { addFile } from "@repo/database/lib/files";
import mime from "mime-types";
import { ensureNodeInstanceId } from "~/utils/nodeInstanceId";
import type { DGSupabaseClient } from "@repo/database/lib/client";
import type { Json } from "@repo/database/dbTypes";
Expand All @@ -9,7 +11,7 @@ import {
type SupabaseContext,
} from "./supabaseContext";
import { default as DiscourseGraphPlugin } from "~/index";
import { publishNode, ensurePublishedRelationsAccuracy } from "./publishNode";
import { ensurePublishedRelationsAccuracy } from "./publishNode";
import { upsertNodesToSupabaseAsContentWithEmbeddings } from "./upsertNodesAsContentWithEmbeddings";
import {
orderConceptsByDependency,
Expand Down Expand Up @@ -579,6 +581,92 @@ const convertDgToSupabaseConcepts = async ({
}
};

export const syncPublishedNodeAssets = async ({
plugin,
client,
nodeId,
spaceId,
file,
attachments,
}: {
plugin: DiscourseGraphPlugin;
client: DGSupabaseClient;
nodeId: string;
spaceId: number;
file: TFile;
attachments?: TFile[];
}): Promise<void> => {
if (attachments === undefined) {
const embeds = plugin.app.metadataCache.getFileCache(file)?.embeds ?? [];
attachments = embeds
.map(({ link }) => {
const attachment = plugin.app.metadataCache.getFirstLinkpathDest(
link,
file.path,
);
return attachment;
})
.filter((a) => !!a);
}
// Always sync non-text assets when node is published to this group
const existingFiles: string[] = [];
const existingReferencesReq = await client
.from("my_file_references")
.select("*")
.eq("space_id", spaceId)
.eq("source_local_id", nodeId);
if (existingReferencesReq.error) {
console.error(existingReferencesReq.error);
return;
}
const existingReferencesByPath = Object.fromEntries(
existingReferencesReq.data.map((ref) => [ref.filepath, ref]),
) as Record<string, (typeof existingReferencesReq.data)[0]>;

for (const attachment of attachments) {
const mimetype = mime.lookup(attachment.path) || "application/octet-stream";
if (mimetype.startsWith("text/")) continue;
// Do not use standard upload for large files
if (attachment.stat.size >= 6 * 1024 * 1024) {
new Notice(
`Asset file ${attachment.path} is larger than 6Mb and will not be uploaded`,
);
continue;
}
existingFiles.push(attachment.path);
const existingRef = existingReferencesByPath[attachment.path];
if (
!existingRef ||
new Date(existingRef.last_modified + "Z").valueOf() <
attachment.stat.mtime
) {
const content = await plugin.app.vault.readBinary(attachment);
await addFile({
client,
spaceId,
sourceLocalId: nodeId,
fname: attachment.path,
mimetype,
created: new Date(attachment.stat.ctime),
lastModified: new Date(attachment.stat.mtime),
content,
});
}
}
let cleanupCommand = client
.from("FileReference")
.delete()
.eq("space_id", spaceId)
.eq("source_local_id", nodeId);
if (existingFiles.length)
cleanupCommand = cleanupCommand.notIn("filepath", [
...new Set(existingFiles),
]);
const cleanupResult = await cleanupCommand;
// do not fail on cleanup
if (cleanupResult.error) console.error(cleanupResult.error);
};

/**
* For nodes that are already published, ensure non-text assets are pushed to
* storage. Called after content sync so new embeds (e.g. images) get uploaded.
Expand All @@ -587,17 +675,26 @@ const syncPublishedNodesAssets = async (
plugin: DiscourseGraphPlugin,
nodes: ObsidianDiscourseNodeData[],
): Promise<void> => {
const context = await getSupabaseContext(plugin);
if (!context) throw new Error("Cannot get context");
const spaceId = context.spaceId;
const client = await getLoggedInClient(plugin);
if (!client) throw new Error("Cannot get client");
const published = nodes.filter(
(n) =>
((n.frontmatter.publishedToGroups as string[] | undefined)?.length ?? 0) >
0,
);
for (const node of published) {
try {
await publishNode({
const nodeId = node.frontmatter.nodeInstanceId as string | undefined;
if (!nodeId) throw new Error("Please sync the node first");
await syncPublishedNodeAssets({
plugin,
client,
nodeId,
spaceId,
file: node.file,
frontmatter: node.frontmatter as FrontMatterCache,
});
} catch (error) {
console.error(
Expand Down Expand Up @@ -651,7 +748,12 @@ const syncChangedNodesToSupabase = async ({

// When file changes affect an already-published node, ensure new non-text
// assets (e.g. images) are pushed to storage.
await syncPublishedNodesAssets(plugin, changedNodes);
try {
await syncPublishedNodesAssets(plugin, changedNodes);
} catch (error) {
console.error(`Failed to sync published node assets`, error);
new Notice(`Failed to sync published node assets.`);
}
Comment on lines +751 to +756
Copy link
Copy Markdown
Contributor

@graphite-app graphite-app bot Apr 12, 2026

Choose a reason for hiding this comment

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

Error is silently swallowed with no logging or user notification. The commented-out error message also references node which doesn't exist in this scope (should be referencing individual nodes from the loop in syncPublishedNodesAssets). This means:

  1. If asset syncing fails for published nodes, users won't be notified
  2. Debugging will be difficult as failures are hidden
  3. If the commented code is ever uncommented, it will throw a ReferenceError

Fix: Either properly log the error or remove the try-catch to let errors propagate:

try {
  await syncPublishedNodesAssets(plugin, changedNodes);
} catch (error) {
  console.error('Failed to sync published node assets:', error);
  // Optionally show a Notice to the user
}
Suggested change
try {
await syncPublishedNodesAssets(plugin, changedNodes);
} catch (error) {
// console.error(
// `Failed to sync published node assets for ${node.file.path}:`,
// error,
// );
}
try {
await syncPublishedNodesAssets(plugin, changedNodes);
} catch (error) {
console.error(
'Failed to sync published node assets:',
error,
);
}

Spotted by Graphite

Fix in Graphite

Is this helpful? React 👍 or 👎 to let us know.

Copy link
Copy Markdown
Collaborator Author

@maparent maparent Apr 12, 2026

Choose a reason for hiding this comment

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

We are indeed swallowing errors silently in the plugin. @trangdoan982 Do you think this one is worth a Notice?

Copy link
Copy Markdown
Member

@trangdoan982 trangdoan982 Apr 14, 2026

Choose a reason for hiding this comment

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

yeah maybe we show them a Notice and ask them to retry?

Copy link
Copy Markdown
Collaborator Author

@maparent maparent Apr 14, 2026

Choose a reason for hiding this comment

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

Added a notice. Not sure we should ask them to retry, unless we have a way to detect multiple failures?

Comment thread
maparent marked this conversation as resolved.
};

/**
Expand Down
Loading