From 239e017e8673366c3dc8f1ff1353180edc001601 Mon Sep 17 00:00:00 2001 From: sid597 Date: Mon, 3 Aug 2026 14:46:20 +0530 Subject: [PATCH 1/5] ENG-2022 Make Roam publishing sync-independent with complete node upserts Add an 'Enable node sharing' feature flag and gate the Publish tab, 'DG: Share current node' command, and page-title Publish button on it instead of the suggestive-mode overlay flag. Publishing now converts selected nodes to full CrossAppNodes (direct title + full markdown) and upserts schema, concept, and contents before granting group access; nodes whose upsert fails are excluded from grants. --- apps/roam/src/components/Export.tsx | 32 +- .../src/components/settings/AdminPanel.tsx | 6 + .../components/settings/utils/accessors.ts | 3 + .../components/settings/utils/settingKeys.ts | 1 + .../settings/utils/zodSchema.example.ts | 2 + .../components/settings/utils/zodSchema.ts | 1 + .../__tests__/nodeSharingFeatureFlag.test.ts | 49 +++ .../__tests__/publishNodesToGroups.test.ts | 289 ++++++++++++++++++ .../utils/initializeObserversAndListeners.ts | 6 +- apps/roam/src/utils/publishNodesToGroups.ts | 99 +++--- .../utils/registerCommandPaletteCommands.ts | 9 +- .../src/utils/roamToCrossAppConverters.ts | 33 +- 12 files changed, 446 insertions(+), 84 deletions(-) create mode 100644 apps/roam/src/utils/__tests__/nodeSharingFeatureFlag.test.ts create mode 100644 apps/roam/src/utils/__tests__/publishNodesToGroups.test.ts diff --git a/apps/roam/src/components/Export.tsx b/apps/roam/src/components/Export.tsx index 6d1b5f071..782493b80 100644 --- a/apps/roam/src/components/Export.tsx +++ b/apps/roam/src/components/Export.tsx @@ -91,7 +91,7 @@ import { type NodeUidWithType, } from "~/utils/publishNodesToGroups"; import { getLoggedInClient, getSupabaseContext } from "~/utils/supabaseContext"; -import { isSyncEnabled } from "~/components/settings/utils/accessors"; +import { isNodeSharingEnabled } from "~/components/settings/utils/accessors"; const ExportProgress = ({ id }: { id: string }) => { const [progress, setProgress] = useState(0); @@ -218,12 +218,12 @@ const ExportDialog: ExportDialogComponent = ({ useState<(typeof SEND_TO_DESTINATIONS)[number]>("page"); const isSendToGraph = activeSendToDestination === "graph"; const [livePages, setLivePages] = useState([]); - const syncEnabled = useMemo(() => isSyncEnabled(), []); + const sharingEnabled = useMemo(() => isNodeSharingEnabled(), []); const [selectedTabId, setSelectedTabId] = useState("sendto"); useEffect(() => { - if (initialPanel === "publish" && !syncEnabled) return; + if (initialPanel === "publish" && !sharingEnabled) return; if (initialPanel) setSelectedTabId(INITIAL_PANEL_TO_TAB_ID[initialPanel]); - }, [initialPanel, syncEnabled]); + }, [initialPanel, sharingEnabled]); const [includeDiscourseContext, setIncludeDiscourseContext] = useState(false); const [gitHubAccessToken, setGitHubAccessToken] = useState( getSetting("oauth-github", null), @@ -240,7 +240,7 @@ const ExportDialog: ExportDialogComponent = ({ const publishableNodes = useMemo( () => - syncEnabled + sharingEnabled ? results .map((r) => { const node = findDiscourseNode({ uid: r.uid }); @@ -250,7 +250,7 @@ const ExportDialog: ExportDialogComponent = ({ }) .filter((n): n is NodeUidWithType => n !== null) : [], - [results, syncEnabled], + [results, sharingEnabled], ); const nonDiscourseCount = results.length - publishableNodes.length; @@ -808,7 +808,7 @@ const ExportDialog: ExportDialogComponent = ({ }; useEffect(() => { if ( - !syncEnabled || + !sharingEnabled || !isOpen || selectedTabId !== "publish" || groupsLoaded || @@ -829,7 +829,7 @@ const ExportDialog: ExportDialogComponent = ({ setGroupsLoaded(true); } })(); - }, [syncEnabled, isOpen, selectedTabId, groupsLoaded, groupsLoading]); + }, [sharingEnabled, isOpen, selectedTabId, groupsLoaded, groupsLoading]); const handlePublish = async () => { setPublishError(""); @@ -840,7 +840,7 @@ const ExportDialog: ExportDialogComponent = ({ if (!client || !context) throw new Error("Could not connect to sync."); const { publishedNodeUids, - skippedUnsyncedUids, + failedSyncedUids, okGroupIds, failedGroupIds, } = await publishNodeUidsWithTypeToGroups({ @@ -852,7 +852,7 @@ const ExportDialog: ExportDialogComponent = ({ posthog.capture("Export Dialog: Publish", { groupCount: okGroupIds.length, publishedNodeCount: publishedNodeUids.length, - skippedUnsyncedCount: skippedUnsyncedUids.length, + failedSyncedCount: failedSyncedUids.length, nonDiscourseCount, failedGroupCount: failedGroupIds.length, }); @@ -866,10 +866,8 @@ const ExportDialog: ExportDialogComponent = ({ }.`, ] : ["No nodes were published."]; - if (skippedUnsyncedUids.length) - messages.push( - `${skippedUnsyncedUids.length} not synced yet — try again shortly.`, - ); + if (failedSyncedUids.length) + messages.push(`${failedSyncedUids.length} failed to publish.`); if (nonDiscourseCount) messages.push(`${nonDiscourseCount} skipped (not discourse nodes).`); if (failedGroupIds.length) @@ -881,7 +879,9 @@ const ExportDialog: ExportDialogComponent = ({ renderToast({ content: messages.join(" "), intent: - failedGroupIds.length || !hasPublishedNodes ? "warning" : "success", + failedGroupIds.length || failedSyncedUids.length || !hasPublishedNodes + ? "warning" + : "success", id: "query-builder-publish-success", }); if (hasPublishedNodes) onClose(); @@ -1263,7 +1263,7 @@ const ExportDialog: ExportDialogComponent = ({ > - {syncEnabled && ( + {sharingEnabled && ( )} diff --git a/apps/roam/src/components/settings/AdminPanel.tsx b/apps/roam/src/components/settings/AdminPanel.tsx index d05cdfe29..4316e3c64 100644 --- a/apps/roam/src/components/settings/AdminPanel.tsx +++ b/apps/roam/src/components/settings/AdminPanel.tsx @@ -356,6 +356,12 @@ const FeatureFlagsTab = (): React.ReactElement => { onAfterChange={(checked) => setAdvancedNodeSearchValue(checked)} /> + + { diff --git a/apps/roam/src/components/settings/utils/accessors.ts b/apps/roam/src/components/settings/utils/accessors.ts index 1c1fa0ad5..2eaa190c8 100644 --- a/apps/roam/src/components/settings/utils/accessors.ts +++ b/apps/roam/src/components/settings/utils/accessors.ts @@ -763,6 +763,9 @@ export const readAllLegacyDiscourseNodeSettings = ( export const isSyncEnabled = (): boolean => getFeatureFlag("Suggestive mode overlay enabled"); +export const isNodeSharingEnabled = (): boolean => + getFeatureFlag("Enable node sharing"); + export const setFeatureFlag = ( key: keyof FeatureFlags, value: boolean, diff --git a/apps/roam/src/components/settings/utils/settingKeys.ts b/apps/roam/src/components/settings/utils/settingKeys.ts index 1dcec7bf0..6278f423e 100644 --- a/apps/roam/src/components/settings/utils/settingKeys.ts +++ b/apps/roam/src/components/settings/utils/settingKeys.ts @@ -13,6 +13,7 @@ import type { export const FEATURE_FLAG_KEYS = { enableLeftSidebar: "Enable left sidebar", + enableNodeSharing: "Enable node sharing", suggestiveModeOverlayEnabled: "Suggestive mode overlay enabled", useNewSettingsStore: "Use new settings store", } as const satisfies Record; diff --git a/apps/roam/src/components/settings/utils/zodSchema.example.ts b/apps/roam/src/components/settings/utils/zodSchema.example.ts index 58fc827da..fc199a676 100644 --- a/apps/roam/src/components/settings/utils/zodSchema.example.ts +++ b/apps/roam/src/components/settings/utils/zodSchema.example.ts @@ -87,6 +87,7 @@ const discourseNodeSettings: DiscourseNodeSettings = { const featureFlags: FeatureFlags = { "Advanced node search enabled": true, "Enable left sidebar": true, + "Enable node sharing": true, "Suggestive mode overlay enabled": true, "Use new settings store": false, }; @@ -94,6 +95,7 @@ const featureFlags: FeatureFlags = { const defaultFeatureFlags: FeatureFlags = { "Advanced node search enabled": false, "Enable left sidebar": false, + "Enable node sharing": false, "Suggestive mode overlay enabled": false, "Use new settings store": false, }; diff --git a/apps/roam/src/components/settings/utils/zodSchema.ts b/apps/roam/src/components/settings/utils/zodSchema.ts index 7f6e0d592..a9793ba3b 100644 --- a/apps/roam/src/components/settings/utils/zodSchema.ts +++ b/apps/roam/src/components/settings/utils/zodSchema.ts @@ -157,6 +157,7 @@ export const DiscourseRelationSchema = z.object({ export const FeatureFlagsSchema = z.object({ "Advanced node search enabled": z.boolean().default(false), "Enable left sidebar": z.boolean().default(false), + "Enable node sharing": z.boolean().default(false), "Suggestive mode overlay enabled": z.boolean().default(false), "Use new settings store": z.boolean().default(false), }); diff --git a/apps/roam/src/utils/__tests__/nodeSharingFeatureFlag.test.ts b/apps/roam/src/utils/__tests__/nodeSharingFeatureFlag.test.ts new file mode 100644 index 000000000..5aefa2f0f --- /dev/null +++ b/apps/roam/src/utils/__tests__/nodeSharingFeatureFlag.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, it, vi } from "vitest"; + +vi.mock("~/utils/internalError", () => ({ default: vi.fn() })); +vi.mock("~/utils/extensionSettings", () => ({ getSetting: vi.fn() })); +vi.mock("~/utils/parseQuery", () => ({ roamNodeToCondition: vi.fn() })); + +import { + isNodeSharingEnabled, + isSyncEnabled, +} from "~/components/settings/utils/accessors"; + +const seedWindow = (featureFlags: Record) => { + (globalThis as { window: unknown }).window = { + roamAlphaAPI: { + user: { uid: () => "user-1" }, + pull: () => ({ + ":block/children": [ + { + ":block/string": "Feature Flags", + ":block/props": { + "Use new settings store": true, + ...featureFlags, + }, + }, + ], + }), + }, + }; +}; + +describe("node sharing feature flag", () => { + it("defaults to disabled alongside suggestive mode", () => { + seedWindow({}); + expect(isNodeSharingEnabled()).toBe(false); + expect(isSyncEnabled()).toBe(false); + }); + + it("enables node sharing without suggestive mode", () => { + seedWindow({ "Enable node sharing": true }); + expect(isNodeSharingEnabled()).toBe(true); + expect(isSyncEnabled()).toBe(false); + }); + + it("does not enable node sharing when only suggestive mode is on", () => { + seedWindow({ "Suggestive mode overlay enabled": true }); + expect(isNodeSharingEnabled()).toBe(false); + expect(isSyncEnabled()).toBe(true); + }); +}); diff --git a/apps/roam/src/utils/__tests__/publishNodesToGroups.test.ts b/apps/roam/src/utils/__tests__/publishNodesToGroups.test.ts new file mode 100644 index 000000000..5984fbd7e --- /dev/null +++ b/apps/roam/src/utils/__tests__/publishNodesToGroups.test.ts @@ -0,0 +1,289 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { CrossAppNode } from "@repo/database/crossAppContracts"; +import type { DGSupabaseClient } from "@repo/database/lib/client"; +import type { DiscourseNode } from "~/utils/getDiscourseNodes"; +import { contentTypes } from "@repo/content-model"; + +const mocks = vi.hoisted(() => ({ + getDiscourseNodes: vi.fn(), + getAvailableGroupIds: vi.fn(), + ensurePartialSpaceAccess: vi.fn(), + internalError: vi.fn(), +})); + +vi.mock("~/utils/getDiscourseNodes", () => ({ + default: mocks.getDiscourseNodes, +})); + +vi.mock("~/utils/getDiscourseRelations", () => ({ + default: () => [], +})); + +vi.mock("~/utils/createReifiedBlock", () => ({ + getReifiedRelations: () => Promise.resolve([]), +})); + +vi.mock("~/utils/internalError", () => ({ + default: mocks.internalError, +})); + +vi.mock("~/utils/importedSourceIdentity", () => ({ + readImportedSourceIdentity: () => undefined, +})); + +vi.mock("~/utils/roamToCrossAppConverters", () => ({ + nodeUidsWithTypeToCrossApp: vi.fn(), + nodeSchemaToCrossApp: (s: DiscourseNode) => ({ + localId: s.type, + label: s.text, + authorId: "author-1", + createdAt: new Date("2026-01-01T00:00:00.000Z"), + }), + reifiedRelationToCrossApp: vi.fn(), + relationTripleSchemaToCrossApp: vi.fn(), +})); + +vi.mock("@repo/database/lib/groups", () => ({ + getAvailableGroupIds: mocks.getAvailableGroupIds, + ensurePartialSpaceAccess: mocks.ensurePartialSpaceAccess, +})); + +vi.mock("@repo/database/lib/contextFunctions", () => ({ + isIgnorableUpsertError: (error: { code?: string } | null) => + !error || error.code === "23505", +})); + +import { publishNodesToGroups } from "~/utils/publishNodesToGroups"; + +const SPACE_ID = 42; +const GROUP_ID = "group-1"; +const SCHEMA_UID = "schema-1"; + +const claimSchema: DiscourseNode = { + type: SCHEMA_UID, + text: "Claim", + shortcut: "C", + specification: [], + backedBy: "user", + canvasSettings: {}, + format: "[[CLM]] - {content}", +}; + +const makeCrossAppNode = ({ + uid, + title, +}: { + uid: string; + title: string; +}): CrossAppNode => ({ + localId: uid, + nodeType: SCHEMA_UID, + authorId: "user-1", + createdAt: new Date("2026-01-02T00:00:00.000Z"), + modifiedAt: new Date("2026-01-03T00:00:00.000Z"), + content: { + direct: { localId: uid, value: title }, + full: { + localId: uid, + value: `# ${title}\n\nBody\n`, + contentType: contentTypes.roamMarkdown, + scale: "document", + }, + }, +}); + +type RpcArgs = { v_space_id: number; data: Record[] }; + +const makeFakeClient = ({ + syncedUids = [], + rpcResponse, +}: { + syncedUids?: string[]; + rpcResponse?: { data: number[] | null; error: { message: string } | null }; +}) => { + const rpcCalls: { fn: string; args: RpcArgs }[] = []; + const upsertCalls: { + table: string; + rows: Record[]; + options: Record; + }[] = []; + const selectResult = (table: string) => + Promise.resolve({ + data: + table === "my_concepts" + ? syncedUids.map((uid) => ({ source_local_id: uid })) + : [], + error: null, + }); + const client = { + from: (table: string) => ({ + select: () => ({ + eq: () => ({ in: () => selectResult(table) }), + in: () => selectResult(table), + }), + upsert: ( + rows: Record[], + options: Record, + ) => { + upsertCalls.push({ table, rows, options }); + return Promise.resolve({ error: null }); + }, + }), + rpc: (fn: string, args: RpcArgs) => { + rpcCalls.push({ fn, args }); + return Promise.resolve( + rpcResponse ?? { data: args.data.map((_, i) => i + 1), error: null }, + ); + }, + } as unknown as DGSupabaseClient; + return { client, rpcCalls, upsertCalls }; +}; + +describe("publishNodesToGroups", () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.getDiscourseNodes.mockReturnValue([claimSchema]); + mocks.getAvailableGroupIds.mockResolvedValue([GROUP_ID]); + mocks.ensurePartialSpaceAccess.mockImplementation( + ({ groupIds }: { groupIds: string[] }) => + Promise.resolve({ + existing: Object.fromEntries(groupIds.map((g) => [g, "partial"])), + missing: {}, + }), + ); + }); + + it("upserts the schema and the complete node concept before granting access to a new node", async () => { + const { client, rpcCalls, upsertCalls } = makeFakeClient({}); + + const result = await publishNodesToGroups({ + client, + spaceId: SPACE_ID, + groupIds: [GROUP_ID], + nodes: [makeCrossAppNode({ uid: "node-1", title: "CLM - new claim" })], + }); + + expect(rpcCalls).toHaveLength(1); + expect(rpcCalls[0].fn).toBe("upsert_concepts"); + const { v_space_id: rpcSpaceId, data } = rpcCalls[0].args; + expect(rpcSpaceId).toBe(SPACE_ID); + expect(data).toHaveLength(2); + expect(data[0]).toMatchObject({ + source_local_id: SCHEMA_UID, + is_schema: true, + name: "Claim", + }); + expect(data[1]).toMatchObject({ + source_local_id: "node-1", + name: "CLM - new claim", + schema_represented_by_local_id: SCHEMA_UID, + }); + expect(data[1].contents_inline).toEqual([ + expect.objectContaining({ + source_local_id: "node-1", + variant: "direct", + text: "CLM - new claim", + }), + expect.objectContaining({ + source_local_id: "node-1", + variant: "full", + text: "# CLM - new claim\n\nBody\n", + content_type: contentTypes.roamMarkdown, + }), + ]); + + expect(upsertCalls).toHaveLength(1); + expect(upsertCalls[0].table).toBe("ResourceAccess"); + expect(upsertCalls[0].options).toEqual({ ignoreDuplicates: true }); + expect(upsertCalls[0].rows).toEqual( + expect.arrayContaining([ + { + account_uid: GROUP_ID, + source_local_id: "node-1", + space_id: SPACE_ID, + }, + { + account_uid: GROUP_ID, + source_local_id: SCHEMA_UID, + space_id: SPACE_ID, + }, + ]), + ); + expect(result.publishedNodeUids).toEqual(["node-1"]); + expect(result.publishedNodeSchemaUids).toEqual([SCHEMA_UID]); + expect(result.syncedNodeSchemaUids).toEqual([SCHEMA_UID]); + expect(result.okGroupIds).toEqual([GROUP_ID]); + expect(result.failedSyncedUids).toEqual([]); + }); + + it("still upserts title and full content when republishing an already-synced node", async () => { + const { client, rpcCalls } = makeFakeClient({ syncedUids: [SCHEMA_UID] }); + + const result = await publishNodesToGroups({ + client, + spaceId: SPACE_ID, + groupIds: [GROUP_ID], + nodes: [ + makeCrossAppNode({ uid: "node-1", title: "CLM - updated title" }), + ], + }); + + const { data } = rpcCalls[0].args; + expect(data).toHaveLength(1); + expect(data[0]).toMatchObject({ source_local_id: "node-1" }); + expect(data[0].contents_inline).toEqual([ + expect.objectContaining({ + variant: "direct", + text: "CLM - updated title", + }), + expect.objectContaining({ + variant: "full", + text: "# CLM - updated title\n\nBody\n", + }), + ]); + expect(result.syncedNodeSchemaUids).toEqual([]); + expect(result.publishedNodeUids).toEqual(["node-1"]); + expect(result.okGroupIds).toEqual([GROUP_ID]); + }); + + it("grants no access at all when the concept upsert request fails", async () => { + const { client, upsertCalls } = makeFakeClient({ + rpcResponse: { data: null, error: { message: "boom" } }, + }); + + const result = await publishNodesToGroups({ + client, + spaceId: SPACE_ID, + groupIds: [GROUP_ID], + nodes: [makeCrossAppNode({ uid: "node-1", title: "CLM - new claim" })], + }); + + expect(upsertCalls).toHaveLength(0); + expect(result.publishedNodeUids).toEqual([]); + expect(result.okGroupIds).toEqual([]); + }); + + it("withholds grants for nodes whose upsert failed", async () => { + const { client, upsertCalls } = makeFakeClient({ + syncedUids: [SCHEMA_UID], + rpcResponse: { data: [7, -1], error: null }, + }); + + const result = await publishNodesToGroups({ + client, + spaceId: SPACE_ID, + groupIds: [GROUP_ID], + nodes: [ + makeCrossAppNode({ uid: "node-1", title: "CLM - publishes" }), + makeCrossAppNode({ uid: "node-2", title: "CLM - fails" }), + ], + }); + + expect(result.failedSyncedUids).toEqual(["node-2"]); + expect(result.publishedNodeUids).toEqual(["node-1"]); + const grantedIds = upsertCalls[0].rows.map((r) => r.source_local_id); + expect(grantedIds).toContain("node-1"); + expect(grantedIds).toContain(SCHEMA_UID); + expect(grantedIds).not.toContain("node-2"); + }); +}); diff --git a/apps/roam/src/utils/initializeObserversAndListeners.ts b/apps/roam/src/utils/initializeObserversAndListeners.ts index c084cf182..a27cc7164 100644 --- a/apps/roam/src/utils/initializeObserversAndListeners.ts +++ b/apps/roam/src/utils/initializeObserversAndListeners.ts @@ -122,9 +122,9 @@ export const initObservers = ({ const isDiscourseNode = node && node.backedBy !== "default"; if (isDiscourseNode) { - const syncEnabled = - settings.featureFlags[FEATURE_FLAG_KEYS.suggestiveModeOverlayEnabled]; - if (syncEnabled && node.backedBy === "user") { + const sharingEnabled = + settings.featureFlags[FEATURE_FLAG_KEYS.enableNodeSharing]; + if (sharingEnabled && node.backedBy === "user") { renderPublishNodeTitleButton({ h1, uid, diff --git a/apps/roam/src/utils/publishNodesToGroups.ts b/apps/roam/src/utils/publishNodesToGroups.ts index b6c27bfb5..0006e900b 100644 --- a/apps/roam/src/utils/publishNodesToGroups.ts +++ b/apps/roam/src/utils/publishNodesToGroups.ts @@ -15,6 +15,7 @@ import getDiscourseRelations from "./getDiscourseRelations"; import { getReifiedRelations } from "./createReifiedBlock"; import { crossAppNodeSchemaToDbConcept, + crossAppNodeToDbConcept, crossAppRelationToDbConcept, crossAppRelationTripleSchemaToDbConcept, } from "@repo/database/lib/crossAppConverters"; @@ -207,19 +208,19 @@ type PublishNodesResult = { syncedRelationTripleSchemaUids: string[]; syncedRelationUids: string[]; failedSyncedUids: string[]; - skippedUnsyncedUids: string[]; okGroupIds: string[]; failedGroupIds: string[]; }; -// Grants a group access to already-synced discourse nodes by mirroring the -// Obsidian publish-to-group access model (SpaceAccess + ResourceAccess), -// without its file/frontmatter/relation/asset coupling. +// Grants a group access to discourse nodes by mirroring the Obsidian +// publish-to-group access model (SpaceAccess + ResourceAccess), without its +// file/frontmatter/asset coupling. // -// ResourceAccess has no foreign key on source_local_id, so granting access to a -// node that has not synced yet would create an orphaned row. We therefore only -// publish nodes confirmed present as instance concepts in this space, and -// report the rest as not-yet-synced (they self-heal on the next sync). +// ResourceAccess has no foreign key on source_local_id, so granting access to +// a node absent from this space would create an orphaned row. We therefore +// upsert every selected node as a complete concept (direct title + full +// content) before granting access, and withhold grants for anything whose +// upsert failed. export const publishNodesToGroups = async ({ client, spaceId, @@ -240,7 +241,6 @@ export const publishNodesToGroups = async ({ syncedRelationUids: [], syncedRelationTripleSchemaUids: [], failedSyncedUids: [], - skippedUnsyncedUids: [], okGroupIds: [], failedGroupIds: [], }; @@ -267,7 +267,8 @@ export const publishNodesToGroups = async ({ } if (groupIds.length === 0) return result; - let nodeUids = [...new Set(nodes.map((node) => node.localId))]; + const nodesByUid = new Map(nodes.map((node) => [node.localId, node])); + let nodeUids = [...nodesByUid.keys()]; const nodeSchemaUids = new Set(nodes.map((node) => node.nodeType)); const nodeSchemas = getDiscourseNodes() .filter((s) => nodeSchemaUids.has(s.type)) @@ -286,7 +287,6 @@ export const publishNodesToGroups = async ({ const neededUids = [ ...nodeSchemaUids, - ...nodeUids, ...relationTripleSchemaUids, ...relationUids, ]; @@ -303,32 +303,24 @@ export const publishNodesToGroups = async ({ const syncedUids = new Set( onlyStrings((syncedRes.data ?? []).map((row) => row.source_local_id)), ); - result.skippedUnsyncedUids = nodeUids.filter((uid) => !syncedUids.has(uid)); - nodeUids = [...intersection(syncedUids, new Set(nodeUids))]; const missingNodeSchemas = nodeSchemas.filter( (s) => !syncedUids.has(s.localId), ); const missingRelationTripleSchemas = relationTripleSchemas.filter( (s) => !syncedUids.has(s.localId), ); - const missingNodeUids = new Set(nodeUids.filter((id) => !syncedUids.has(id))); - const relationsWithSyncedNodes = relations.filter( - (r) => - !missingNodeUids.has(r.source) && !missingNodeUids.has(r.destination), - ); - const missingRelations = relationsWithSyncedNodes.filter( - (r) => !syncedUids.has(r.localId), - ); + const missingRelations = relations.filter((r) => !syncedUids.has(r.localId)); - result.skippedUnsyncedUids = nodeUids.filter((uid) => !syncedUids.has(uid)); const upsertConcepts = [ ...missingNodeSchemas.map((s) => crossAppNodeSchemaToDbConcept(s)), + ...[...nodesByUid.values()].map((node) => crossAppNodeToDbConcept(node)), ...missingRelationTripleSchemas.map((rs3) => crossAppRelationTripleSchemaToDbConcept(rs3), ), ...missingRelations.map((r) => crossAppRelationToDbConcept(r)), ].filter((r) => r !== undefined); + const upsertedNodeUids = new Set(nodeUids); const syncedRelationUids = new Set(missingRelations.map((s) => s.localId)); const syncedRelationTripleSchemaUids = new Set( missingRelationTripleSchemas.map((s) => s.localId), @@ -337,44 +329,49 @@ export const publishNodesToGroups = async ({ missingNodeSchemas.map((s) => s.localId), ); - if (upsertConcepts.length > 0) { - const response = await client.rpc("upsert_concepts", { - v_space_id: spaceId, - data: upsertConcepts, - }); - if (response.error) { - internalError({ error: response.error }); - return result; - } + const response = await client.rpc("upsert_concepts", { + v_space_id: spaceId, + data: upsertConcepts, + }); + if (response.error) { + internalError({ error: response.error }); + return result; + } - response.data.forEach((v, i) => { - if (v === -1) { - const localId = upsertConcepts[i].source_local_id; - if (localId) { - if (syncedNodeSchemaUids.has(localId)) { - syncedNodeSchemaUids.delete(localId); - nodeSchemaUids.delete(localId); - } else if (syncedRelationTripleSchemaUids.has(localId)) { - syncedRelationTripleSchemaUids.delete(localId); - } else if (syncedRelationUids.has(localId)) { - syncedRelationUids.delete(localId); - } - result.failedSyncedUids.push(localId); + response.data.forEach((v, i) => { + if (v === -1) { + const localId = upsertConcepts[i].source_local_id; + if (localId) { + if (syncedNodeSchemaUids.has(localId)) { + syncedNodeSchemaUids.delete(localId); + nodeSchemaUids.delete(localId); + } else if (upsertedNodeUids.has(localId)) { + upsertedNodeUids.delete(localId); + } else if (syncedRelationTripleSchemaUids.has(localId)) { + syncedRelationTripleSchemaUids.delete(localId); + } else if (syncedRelationUids.has(localId)) { + syncedRelationUids.delete(localId); } + result.failedSyncedUids.push(localId); } - }); - result.syncedNodeSchemaUids = [...syncedNodeSchemaUids]; - result.syncedRelationTripleSchemaUids = [...syncedRelationTripleSchemaUids]; - result.syncedRelationUids = [...syncedRelationUids]; - } + } + }); + result.syncedNodeSchemaUids = [...syncedNodeSchemaUids]; + result.syncedRelationTripleSchemaUids = [...syncedRelationTripleSchemaUids]; + result.syncedRelationUids = [...syncedRelationUids]; + nodeUids = [...upsertedNodeUids]; const failedSyncIds = new Set(result.failedSyncedUids); const resourceAccesses = []; const resourceIds = [...nodeUids, ...nodeSchemaUids]; for (const groupId of groupIds) { let groupRelationIds = new Set(relevantRelationIdsPerGroupId[groupId]); - const groupRelations = relationsWithSyncedNodes.filter( - (r) => groupRelationIds.has(r.localId) && !failedSyncIds.has(r.localId), + const groupRelations = relations.filter( + (r) => + groupRelationIds.has(r.localId) && + !failedSyncIds.has(r.localId) && + !failedSyncIds.has(r.source) && + !failedSyncIds.has(r.destination), ); groupRelationIds = new Set(groupRelations.map((r) => r.localId)); const groupRelationTripleSchemaIds = new Set( diff --git a/apps/roam/src/utils/registerCommandPaletteCommands.ts b/apps/roam/src/utils/registerCommandPaletteCommands.ts index 11d55ba38..140488177 100644 --- a/apps/roam/src/utils/registerCommandPaletteCommands.ts +++ b/apps/roam/src/utils/registerCommandPaletteCommands.ts @@ -31,6 +31,7 @@ import { setPersonalSetting, setGlobalSetting, isSyncEnabled, + isNodeSharingEnabled, } from "~/components/settings/utils/accessors"; import { DISCOURSE_NODE_KEYS, @@ -248,10 +249,10 @@ export const registerCommandPaletteCommands = (onloadArgs: OnloadArgs) => { }; const shareCurrentNode = () => { - if (!isSyncEnabled()) { + if (!isNodeSharingEnabled()) { renderToast({ - id: "share-node-sync-disabled", - content: "Sync must be enabled to publish discourse nodes.", + id: "share-node-sharing-disabled", + content: "Node sharing must be enabled to publish discourse nodes.", }); return; } @@ -422,6 +423,8 @@ export const registerCommandPaletteCommands = (onloadArgs: OnloadArgs) => { void addCommand("DG: Open - Discourse settings", renderSettingsPopup); if (isSyncEnabled()) { void addCommand("DG: Discover shared nodes", discoverSharedNodes); + } + if (isNodeSharingEnabled()) { void addCommand("DG: Share current node", shareCurrentNode); } if (getFeatureFlag("Advanced node search enabled")) { diff --git a/apps/roam/src/utils/roamToCrossAppConverters.ts b/apps/roam/src/utils/roamToCrossAppConverters.ts index acfa75b05..e756e77db 100644 --- a/apps/roam/src/utils/roamToCrossAppConverters.ts +++ b/apps/roam/src/utils/roamToCrossAppConverters.ts @@ -45,13 +45,27 @@ export const buildFullMarkdown = ({ return body ? `# ${title}\n\n${body}\n` : `# ${title}\n`; }; +const buildFullInlineContent = ({ + uid, + title, +}: { + uid: string; + title: string; +}): NonNullable => { + const blocks = getFullTreeByParentUid(uid).children; + const viewType = getPageViewType(title) || "bullet"; + return { + localId: uid, + value: buildFullMarkdown({ title, blocks, viewType }), + contentType: contentTypes.roamMarkdown, + scale: "document", + }; +}; + export const fullContentNodeToCrossApp = ( node: RoamFullContentNode, ): CrossAppNode => { const title = node.node_title ?? node.text; - const blocks = getFullTreeByParentUid(node.source_local_id).children; - const viewType = getPageViewType(title) || "bullet"; - const fullText = buildFullMarkdown({ title, blocks, viewType }); return { authorId: node.author_local_id, @@ -62,14 +76,9 @@ export const fullContentNodeToCrossApp = ( content: { direct: { localId: node.source_local_id, - value: node.node_title ?? node.text, - }, - full: { - localId: node.source_local_id, - value: fullText, - contentType: contentTypes.roamMarkdown, - scale: "document", + value: title, }, + full: buildFullInlineContent({ uid: node.source_local_id, title }), }, }; }; @@ -99,6 +108,7 @@ export const nodeUidsWithTypeToCrossApp = async ( ); const results = nodeRows.map((row) => { const uid = row[":block/uid"] as string; + const title = row[":node/title"] as string; const userUid = userUidByEid[(row[":create/user"] as Record)[":db/id"]]; @@ -116,8 +126,9 @@ export const nodeUidsWithTypeToCrossApp = async ( content: { direct: { localId: uid, - value: row[":node/title"] as string, + value: title, }, + full: buildFullInlineContent({ uid, title }), }, }; }); From 61759a9b84d9eec3f8d758edcb5cf5101695a292 Mon Sep 17 00:00:00 2001 From: sid597 Date: Fri, 7 Aug 2026 14:14:25 +0530 Subject: [PATCH 2/5] ENG-2022 Treat all negative upsert codes as failures and stop fabricating publish timestamps upsert_concepts returns -1 for unique violations and -2 for other errors; only -1 was treated as a failure, so a -2 node kept its ResourceAccess grant. Node timestamps now follow the sync query's fallback chain (create -> edit -> page-edit) instead of falling back to Date.now(), which would have persisted publish time as last_modified whenever :page/edit-time was absent. failedSyncedUids is renamed to failedUpsertUids and the publish toast counts only failed selected nodes. --- apps/roam/src/components/Export.tsx | 14 +++-- .../__tests__/publishNodesToGroups.test.ts | 50 ++++++++------- .../roamToCrossAppConverters.test.ts | 62 +++++++++++++++++++ apps/roam/src/utils/publishNodesToGroups.ts | 18 +++--- .../src/utils/roamToCrossAppConverters.ts | 13 ++-- 5 files changed, 114 insertions(+), 43 deletions(-) create mode 100644 apps/roam/src/utils/__tests__/roamToCrossAppConverters.test.ts diff --git a/apps/roam/src/components/Export.tsx b/apps/roam/src/components/Export.tsx index 782493b80..0f1c350fb 100644 --- a/apps/roam/src/components/Export.tsx +++ b/apps/roam/src/components/Export.tsx @@ -840,7 +840,7 @@ const ExportDialog: ExportDialogComponent = ({ if (!client || !context) throw new Error("Could not connect to sync."); const { publishedNodeUids, - failedSyncedUids, + failedUpsertUids, okGroupIds, failedGroupIds, } = await publishNodeUidsWithTypeToGroups({ @@ -849,10 +849,14 @@ const ExportDialog: ExportDialogComponent = ({ groupIds: selectedGroupIds, nodeUids: publishableNodes, }); + const selectedNodeUids = new Set(publishableNodes.map((n) => n.uid)); + const failedNodeCount = failedUpsertUids.filter((uid) => + selectedNodeUids.has(uid), + ).length; posthog.capture("Export Dialog: Publish", { groupCount: okGroupIds.length, publishedNodeCount: publishedNodeUids.length, - failedSyncedCount: failedSyncedUids.length, + failedUpsertCount: failedUpsertUids.length, nonDiscourseCount, failedGroupCount: failedGroupIds.length, }); @@ -866,8 +870,8 @@ const ExportDialog: ExportDialogComponent = ({ }.`, ] : ["No nodes were published."]; - if (failedSyncedUids.length) - messages.push(`${failedSyncedUids.length} failed to publish.`); + if (failedNodeCount) + messages.push(`${failedNodeCount} failed to publish.`); if (nonDiscourseCount) messages.push(`${nonDiscourseCount} skipped (not discourse nodes).`); if (failedGroupIds.length) @@ -879,7 +883,7 @@ const ExportDialog: ExportDialogComponent = ({ renderToast({ content: messages.join(" "), intent: - failedGroupIds.length || failedSyncedUids.length || !hasPublishedNodes + failedGroupIds.length || failedNodeCount || !hasPublishedNodes ? "warning" : "success", id: "query-builder-publish-success", diff --git a/apps/roam/src/utils/__tests__/publishNodesToGroups.test.ts b/apps/roam/src/utils/__tests__/publishNodesToGroups.test.ts index 5984fbd7e..be98089a1 100644 --- a/apps/roam/src/utils/__tests__/publishNodesToGroups.test.ts +++ b/apps/roam/src/utils/__tests__/publishNodesToGroups.test.ts @@ -213,7 +213,7 @@ describe("publishNodesToGroups", () => { expect(result.publishedNodeSchemaUids).toEqual([SCHEMA_UID]); expect(result.syncedNodeSchemaUids).toEqual([SCHEMA_UID]); expect(result.okGroupIds).toEqual([GROUP_ID]); - expect(result.failedSyncedUids).toEqual([]); + expect(result.failedUpsertUids).toEqual([]); }); it("still upserts title and full content when republishing an already-synced node", async () => { @@ -263,27 +263,33 @@ describe("publishNodesToGroups", () => { expect(result.okGroupIds).toEqual([]); }); - it("withholds grants for nodes whose upsert failed", async () => { - const { client, upsertCalls } = makeFakeClient({ - syncedUids: [SCHEMA_UID], - rpcResponse: { data: [7, -1], error: null }, - }); + it.each([ + { code: -1, label: "unique-violation" }, + { code: -2, label: "generic-error" }, + ])( + "withholds grants for nodes whose upsert failed with $label", + async ({ code }) => { + const { client, upsertCalls } = makeFakeClient({ + syncedUids: [SCHEMA_UID], + rpcResponse: { data: [7, code], error: null }, + }); - const result = await publishNodesToGroups({ - client, - spaceId: SPACE_ID, - groupIds: [GROUP_ID], - nodes: [ - makeCrossAppNode({ uid: "node-1", title: "CLM - publishes" }), - makeCrossAppNode({ uid: "node-2", title: "CLM - fails" }), - ], - }); + const result = await publishNodesToGroups({ + client, + spaceId: SPACE_ID, + groupIds: [GROUP_ID], + nodes: [ + makeCrossAppNode({ uid: "node-1", title: "CLM - publishes" }), + makeCrossAppNode({ uid: "node-2", title: "CLM - fails" }), + ], + }); - expect(result.failedSyncedUids).toEqual(["node-2"]); - expect(result.publishedNodeUids).toEqual(["node-1"]); - const grantedIds = upsertCalls[0].rows.map((r) => r.source_local_id); - expect(grantedIds).toContain("node-1"); - expect(grantedIds).toContain(SCHEMA_UID); - expect(grantedIds).not.toContain("node-2"); - }); + expect(result.failedUpsertUids).toEqual(["node-2"]); + expect(result.publishedNodeUids).toEqual(["node-1"]); + const grantedIds = upsertCalls[0].rows.map((r) => r.source_local_id); + expect(grantedIds).toContain("node-1"); + expect(grantedIds).toContain(SCHEMA_UID); + expect(grantedIds).not.toContain("node-2"); + }, + ); }); diff --git a/apps/roam/src/utils/__tests__/roamToCrossAppConverters.test.ts b/apps/roam/src/utils/__tests__/roamToCrossAppConverters.test.ts new file mode 100644 index 000000000..5596eb602 --- /dev/null +++ b/apps/roam/src/utils/__tests__/roamToCrossAppConverters.test.ts @@ -0,0 +1,62 @@ +import { describe, expect, it, vi } from "vitest"; +import type { Json } from "@repo/database/dbTypes"; + +vi.mock("roamjs-components/queries/getFullTreeByParentUid", () => ({ + default: () => ({ children: [] }), +})); +vi.mock("roamjs-components/queries/getPageViewType", () => ({ + default: () => "bullet", +})); +vi.mock("~/utils/pageToMarkdown", () => ({ toMarkdown: () => "" })); + +import { nodeUidsWithTypeToCrossApp } from "~/utils/roamToCrossAppConverters"; + +const USER_ROW = { ":db/id": 5, ":user/uid": "user-1" }; + +const convertRow = async (row: Record) => { + (globalThis as { window: unknown }).window = { + roamAlphaAPI: { + data: { + async: { + pull_many: vi + .fn() + .mockResolvedValueOnce([row]) + .mockResolvedValueOnce([USER_ROW]), + }, + }, + }, + }; + const [node] = await nodeUidsWithTypeToCrossApp([ + { uid: "node-1", type: "schema-1" }, + ]); + return node; +}; + +const baseRow = { + ":block/uid": "node-1", + ":node/title": "CLM - claim", + ":create/user": { ":db/id": 5 }, + ":create/time": 1000, +}; + +describe("nodeUidsWithTypeToCrossApp timestamps", () => { + it("uses the page edit time when present", async () => { + const node = await convertRow({ + ...baseRow, + ":edit/time": 2000, + ":page/edit-time": 3000, + }); + expect(node.createdAt).toEqual(new Date(1000)); + expect(node.modifiedAt).toEqual(new Date(3000)); + }); + + it("falls back to the node edit time when page edit time is absent", async () => { + const node = await convertRow({ ...baseRow, ":edit/time": 2000 }); + expect(node.modifiedAt).toEqual(new Date(2000)); + }); + + it("falls back to the create time when no edit time exists", async () => { + const node = await convertRow(baseRow); + expect(node.modifiedAt).toEqual(new Date(1000)); + }); +}); diff --git a/apps/roam/src/utils/publishNodesToGroups.ts b/apps/roam/src/utils/publishNodesToGroups.ts index 0006e900b..b52590634 100644 --- a/apps/roam/src/utils/publishNodesToGroups.ts +++ b/apps/roam/src/utils/publishNodesToGroups.ts @@ -207,7 +207,7 @@ type PublishNodesResult = { syncedNodeSchemaUids: string[]; syncedRelationTripleSchemaUids: string[]; syncedRelationUids: string[]; - failedSyncedUids: string[]; + failedUpsertUids: string[]; okGroupIds: string[]; failedGroupIds: string[]; }; @@ -240,7 +240,7 @@ export const publishNodesToGroups = async ({ syncedNodeSchemaUids: [], syncedRelationUids: [], syncedRelationTripleSchemaUids: [], - failedSyncedUids: [], + failedUpsertUids: [], okGroupIds: [], failedGroupIds: [], }; @@ -339,7 +339,7 @@ export const publishNodesToGroups = async ({ } response.data.forEach((v, i) => { - if (v === -1) { + if (v < 0) { const localId = upsertConcepts[i].source_local_id; if (localId) { if (syncedNodeSchemaUids.has(localId)) { @@ -352,7 +352,7 @@ export const publishNodesToGroups = async ({ } else if (syncedRelationUids.has(localId)) { syncedRelationUids.delete(localId); } - result.failedSyncedUids.push(localId); + result.failedUpsertUids.push(localId); } } }); @@ -360,7 +360,7 @@ export const publishNodesToGroups = async ({ result.syncedRelationTripleSchemaUids = [...syncedRelationTripleSchemaUids]; result.syncedRelationUids = [...syncedRelationUids]; nodeUids = [...upsertedNodeUids]; - const failedSyncIds = new Set(result.failedSyncedUids); + const failedUpsertIds = new Set(result.failedUpsertUids); const resourceAccesses = []; const resourceIds = [...nodeUids, ...nodeSchemaUids]; @@ -369,15 +369,15 @@ export const publishNodesToGroups = async ({ const groupRelations = relations.filter( (r) => groupRelationIds.has(r.localId) && - !failedSyncIds.has(r.localId) && - !failedSyncIds.has(r.source) && - !failedSyncIds.has(r.destination), + !failedUpsertIds.has(r.localId) && + !failedUpsertIds.has(r.source) && + !failedUpsertIds.has(r.destination), ); groupRelationIds = new Set(groupRelations.map((r) => r.localId)); const groupRelationTripleSchemaIds = new Set( groupRelations .map((r) => r.relationType) - .filter((r) => !failedSyncIds.has(r)), + .filter((r) => !failedUpsertIds.has(r)), ); const groupResourceIds = [ ...resourceIds, diff --git a/apps/roam/src/utils/roamToCrossAppConverters.ts b/apps/roam/src/utils/roamToCrossAppConverters.ts index e756e77db..a3c33f399 100644 --- a/apps/roam/src/utils/roamToCrossAppConverters.ts +++ b/apps/roam/src/utils/roamToCrossAppConverters.ts @@ -111,18 +111,17 @@ export const nodeUidsWithTypeToCrossApp = async ( const title = row[":node/title"] as string; const userUid = userUidByEid[(row[":create/user"] as Record)[":db/id"]]; + const createdTime = row[":create/time"] as number; + const editTime = (row[":edit/time"] as number | undefined) ?? createdTime; + const pageEditTime = + (row[":page/edit-time"] as number | undefined) ?? editTime; return { localId: uid, nodeType: typesByUid[uid], authorId: userUid, - createdAt: new Date((row[":create/time"] as number) || Date.now()), - modifiedAt: new Date( - Math.max( - row[":edit/time"] as number, - row[":page/edit-time"] as number, - ) || Date.now(), - ), + createdAt: new Date(createdTime), + modifiedAt: new Date(Math.max(editTime, pageEditTime)), content: { direct: { localId: uid, From 949495a71113242300ee1dc6302309c6fba5e032 Mon Sep 17 00:00:00 2001 From: sid597 Date: Fri, 7 Aug 2026 15:53:52 +0530 Subject: [PATCH 3/5] ENG-2022 Run background sync for shared nodes when node sharing is on without sync The sync loop now also starts when only node sharing is enabled. In that mode it scopes node upserts to shared nodes and uploads their content without generating embeddings; users, shared full-content refresh, concept conversion, and orphan cleanup run as before. With the sync flag on, behavior is unchanged. --- .../src/components/settings/AdminPanel.tsx | 2 +- apps/roam/src/index.ts | 3 +- apps/roam/src/utils/syncDgNodesToSupabase.ts | 37 ++++++++++++++++--- 3 files changed, 34 insertions(+), 8 deletions(-) diff --git a/apps/roam/src/components/settings/AdminPanel.tsx b/apps/roam/src/components/settings/AdminPanel.tsx index 4316e3c64..da19cd120 100644 --- a/apps/roam/src/components/settings/AdminPanel.tsx +++ b/apps/roam/src/components/settings/AdminPanel.tsx @@ -358,7 +358,7 @@ const FeatureFlagsTab = (): React.ReactElement => { diff --git a/apps/roam/src/index.ts b/apps/roam/src/index.ts index 8149fb188..6333a4465 100644 --- a/apps/roam/src/index.ts +++ b/apps/roam/src/index.ts @@ -37,6 +37,7 @@ import { initPostHog } from "./utils/posthog"; import { initSchema } from "./components/settings/utils/init"; import { bulkReadSettings, + isNodeSharingEnabled, isSyncEnabled, } from "./components/settings/utils/accessors"; import { PERSONAL_KEYS } from "./components/settings/utils/settingKeys"; @@ -134,7 +135,7 @@ export default runExtension(async (onloadArgs) => { document.addEventListener("input", discourseNodeSearchTriggerListener); document.addEventListener("selectionchange", nodeCreationPopoverListener); - if (isSyncEnabled()) { + if (isSyncEnabled() || isNodeSharingEnabled()) { initializeSupabaseSync(); } diff --git a/apps/roam/src/utils/syncDgNodesToSupabase.ts b/apps/roam/src/utils/syncDgNodesToSupabase.ts index 85d6c12e0..b3bd20e2f 100644 --- a/apps/roam/src/utils/syncDgNodesToSupabase.ts +++ b/apps/roam/src/utils/syncDgNodesToSupabase.ts @@ -28,6 +28,7 @@ import { intersection } from "@repo/utils/setOperations"; import type { Json, Enums } from "@repo/database/dbTypes"; import { render as renderToast } from "roamjs-components/components/Toast"; import internalError from "~/utils/internalError"; +import { isSyncEnabled } from "~/components/settings/utils/accessors"; import { FatalError } from "@repo/database/lib/contextFunctions"; import { getAllPages } from "@repo/database/lib/pagination"; import type { @@ -759,6 +760,18 @@ export const upsertNodesToSupabaseAsContentWithEmbeddings = async ( }); }; +const upsertNodesToSupabaseAsContent = async ( + roamNodes: RoamDiscourseNodeData[], + supabaseClient: DGSupabaseClient, + context: SupabaseContext, +): Promise => { + if (roamNodes.length === 0) { + return; + } + const content = convertRoamNodeToLocalContent({ nodes: roamNodes }); + await uploadContentBatches({ content, supabaseClient, context }); +}; + const upsertRoamNodesToSupabaseAsFullContent = async ({ nodes, supabaseClient, @@ -1193,6 +1206,12 @@ export const createOrUpdateDiscourseEmbedding = async ( spaceId: activeContext.spaceId, }), }); + const sharedNodesOnlySync = !isSyncEnabled(); + const nodeInstancesToSync = sharedNodesOnlySync + ? changedNodeInstances.filter((node) => + sharedSourceLocalIds.has(node.source_local_id), + ) + : changedNodeInstances; const sharedSourceLocalIdsToBackfill = await measureSyncPhase({ phase: "getSharedSourceLocalIdsMissingFullContent", phases, @@ -1244,11 +1263,17 @@ export const createOrUpdateDiscourseEmbedding = async ( phase: "upsertNodes", phases, operation: () => - upsertNodesToSupabaseAsContentWithEmbeddings( - changedNodeInstances, - activeSupabaseClient, - activeContext, - ), + sharedNodesOnlySync + ? upsertNodesToSupabaseAsContent( + nodeInstancesToSync, + activeSupabaseClient, + activeContext, + ) + : upsertNodesToSupabaseAsContentWithEmbeddings( + nodeInstancesToSync, + activeSupabaseClient, + activeContext, + ), }); await measureSyncPhase({ phase: "upsertFullContent", @@ -1265,7 +1290,7 @@ export const createOrUpdateDiscourseEmbedding = async ( phases, operation: () => convertDgToSupabaseConcepts({ - nodesSince: changedNodeInstances, + nodesSince: nodeInstancesToSync, since: sinceTime, allNodeTypes: allDgNodeTypes, sharedNodeTypeIds, From b3df7747516803b226bc3772f28f49843df5960d Mon Sep 17 00:00:00 2001 From: sid597 Date: Fri, 7 Aug 2026 16:55:08 +0530 Subject: [PATCH 4/5] ENG-2022 Claim a separate shared-content sync task when sync is disabled Shared-nodes-only cycles previously completed the same "embedding" sync task, advancing its watermark without producing embeddings. Enabling the sync flag later would then skip the initial embedding backfill for nodes whose content was already uploaded. Keeping the two modes on separate sync_info rows leaves the embedding watermark untouched until full sync actually runs, and stops the two modes from postponing each other's cycles in mixed-flag spaces. --- apps/roam/src/utils/syncDgNodesToSupabase.ts | 45 ++++++++++++++------ 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/apps/roam/src/utils/syncDgNodesToSupabase.ts b/apps/roam/src/utils/syncDgNodesToSupabase.ts index b3bd20e2f..5427398e9 100644 --- a/apps/roam/src/utils/syncDgNodesToSupabase.ts +++ b/apps/roam/src/utils/syncDgNodesToSupabase.ts @@ -39,6 +39,7 @@ import type { import type { Properties } from "posthog-js"; const SYNC_FUNCTION = "embedding"; +const SHARED_CONTENT_SYNC_FUNCTION = "shared-content"; // Minimal interval between syncs of all clients for this task. const SYNC_INTERVAL = "130s"; // Interval between syncs for each client individually @@ -192,6 +193,7 @@ const syncTelemetryContext = ({ attemptId, worker, userUid, + syncFunction, context, startTime, claimed, @@ -205,6 +207,7 @@ const syncTelemetryContext = ({ attemptId: string; worker: string; userUid: string; + syncFunction: string; context: SupabaseContext | null; startTime: Date; claimed: boolean; @@ -226,7 +229,7 @@ const syncTelemetryContext = ({ return { syncAttemptId: attemptId, syncWorkerId: worker, - syncFunction: SYNC_FUNCTION, + syncFunction, syncUserUid: userUid, claimed, status, @@ -322,7 +325,6 @@ const notifyEndSyncFailure = ({ error, type: "Sync Failed", context: { - syncFunction: SYNC_FUNCTION, status, reason, ...(context || {}), @@ -334,6 +336,7 @@ const notifyEndSyncFailure = ({ export const endSyncTask = async ({ worker, + syncFunction, status, showToast = false, taskStartedAt, @@ -342,6 +345,7 @@ export const endSyncTask = async ({ telemetryContext, }: { worker: string; + syncFunction: string; status: Enums<"task_status">; showToast: boolean; taskStartedAt: Date; @@ -353,7 +357,7 @@ export const endSyncTask = async ({ resolvedContext?: SupabaseContext, ): Properties => ({ syncWorkerId: worker, - syncFunction: SYNC_FUNCTION, + syncFunction, spaceId: resolvedContext?.spaceId ?? context?.spaceId, ...(telemetryContext || {}), }); @@ -384,7 +388,7 @@ export const endSyncTask = async ({ } const { data, error } = await resolvedClient.rpc("end_sync_task", { s_target: resolvedContext.spaceId, - s_function: SYNC_FUNCTION, + s_function: syncFunction, s_worker: worker, s_status: status, s_started_at: taskStartedAt.toISOString(), @@ -502,16 +506,22 @@ export const endSyncTask = async ({ } }; -export const proposeSyncTask = async ( - worker: string, - supabaseClient: DGSupabaseClient, - context: SupabaseContext, -): Promise => { +export const proposeSyncTask = async ({ + worker, + syncFunction, + supabaseClient, + context, +}: { + worker: string; + syncFunction: string; + supabaseClient: DGSupabaseClient; + context: SupabaseContext; +}): Promise => { try { const now = new Date(); const { data, error } = await supabaseClient.rpc("propose_sync_task", { s_target: context.spaceId, - s_function: SYNC_FUNCTION, + s_function: syncFunction, s_worker: worker, task_interval: SYNC_INTERVAL, timeout: SYNC_TIMEOUT, @@ -1077,6 +1087,10 @@ export const createOrUpdateDiscourseEmbedding = async ( let failureReason: string | undefined; let failureContext: Properties | undefined; const worker = getSyncWorkerId(); + const sharedNodesOnlySync = !isSyncEnabled(); + const syncFunction = sharedNodesOnlySync + ? SHARED_CONTENT_SYNC_FUNCTION + : SYNC_FUNCTION; const buildTelemetry = ({ status, @@ -1095,6 +1109,7 @@ export const createOrUpdateDiscourseEmbedding = async ( attemptId, worker, userUid, + syncFunction, context, startTime, claimed, @@ -1138,7 +1153,12 @@ export const createOrUpdateDiscourseEmbedding = async ( phase: "proposeSyncTask", phases, operation: () => - proposeSyncTask(worker, activeSupabaseClient, activeContext), + proposeSyncTask({ + worker, + syncFunction, + supabaseClient: activeSupabaseClient, + context: activeContext, + }), }); if (!shouldProceed) { if (nextUpdateTime === undefined) { @@ -1206,7 +1226,6 @@ export const createOrUpdateDiscourseEmbedding = async ( spaceId: activeContext.spaceId, }), }); - const sharedNodesOnlySync = !isSyncEnabled(); const nodeInstancesToSync = sharedNodesOnlySync ? changedNodeInstances.filter((node) => sharedSourceLocalIds.has(node.source_local_id), @@ -1310,6 +1329,7 @@ export const createOrUpdateDiscourseEmbedding = async ( operation: () => endSyncTask({ worker, + syncFunction, status: "complete", showToast, taskStartedAt: activeClaimedAt, @@ -1377,6 +1397,7 @@ export const createOrUpdateDiscourseEmbedding = async ( operation: () => endSyncTask({ worker, + syncFunction, status: "failed", showToast, taskStartedAt: failedClaimedAt, From accf8197a92c549172cfb9d9409cca1d4c5bd1b8 Mon Sep 17 00:00:00 2001 From: sid597 Date: Fri, 7 Aug 2026 19:33:16 +0530 Subject: [PATCH 5/5] ENG-2022 Withhold grants for nodes whose schema upsert failed A failed schema upsert leaves its concept absent, so dependent nodes insert with a null schema_id and shared-node discovery filters them out. Treat those nodes as failed too: no ResourceAccess grant, counted in the failure toast, and relations touching them are withheld. --- .../__tests__/publishNodesToGroups.test.ts | 21 +++++++++++++++++++ apps/roam/src/utils/publishNodesToGroups.ts | 9 ++++++++ 2 files changed, 30 insertions(+) diff --git a/apps/roam/src/utils/__tests__/publishNodesToGroups.test.ts b/apps/roam/src/utils/__tests__/publishNodesToGroups.test.ts index be98089a1..46dae6534 100644 --- a/apps/roam/src/utils/__tests__/publishNodesToGroups.test.ts +++ b/apps/roam/src/utils/__tests__/publishNodesToGroups.test.ts @@ -292,4 +292,25 @@ describe("publishNodesToGroups", () => { expect(grantedIds).not.toContain("node-2"); }, ); + + it("withholds dependent nodes when their schema upsert fails", async () => { + const { client, upsertCalls } = makeFakeClient({ + rpcResponse: { data: [-1, 2, 3], error: null }, + }); + + const result = await publishNodesToGroups({ + client, + spaceId: SPACE_ID, + groupIds: [GROUP_ID], + nodes: [ + makeCrossAppNode({ uid: "node-1", title: "CLM - first" }), + makeCrossAppNode({ uid: "node-2", title: "CLM - second" }), + ], + }); + + expect(result.failedUpsertUids).toEqual([SCHEMA_UID, "node-1", "node-2"]); + expect(result.publishedNodeUids).toEqual([]); + expect(result.publishedNodeSchemaUids).toEqual([]); + expect(upsertCalls[0].rows).toEqual([]); + }); }); diff --git a/apps/roam/src/utils/publishNodesToGroups.ts b/apps/roam/src/utils/publishNodesToGroups.ts index b52590634..8b60547ce 100644 --- a/apps/roam/src/utils/publishNodesToGroups.ts +++ b/apps/roam/src/utils/publishNodesToGroups.ts @@ -356,6 +356,15 @@ export const publishNodesToGroups = async ({ } } }); + for (const node of nodesByUid.values()) { + if ( + !nodeSchemaUids.has(node.nodeType) && + upsertedNodeUids.has(node.localId) + ) { + upsertedNodeUids.delete(node.localId); + result.failedUpsertUids.push(node.localId); + } + } result.syncedNodeSchemaUids = [...syncedNodeSchemaUids]; result.syncedRelationTripleSchemaUids = [...syncedRelationTripleSchemaUids]; result.syncedRelationUids = [...syncedRelationUids];