Skip to content

ENG-1865 Publish Roam stored relations for shared nodes #1221

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

Draft
maparent wants to merge 1 commit into
base: main
Choose a base branch
from
+451 −42
Draft

ENG-1865 Publish Roam stored relations for shared nodes #1221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
30 changes: 30 additions & 0 deletions apps/roam/src/utils/createReifiedBlock.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,36 @@ export const countReifiedRelations = async (): Promise<number> => {
return (r[0] || [0])[0] as number;
};

export type ReifiedRelationData = {
sourceUid: string;
destinationUid: string;
hasSchema: string;
importedFromRid?: string;
};

export type ReifiedRelationDataWithRelId = ReifiedRelationData & {
relationId: string;
};

export const getReifiedRelations = async (): Promise<
ReifiedRelationDataWithRelId[]
> => {
const pageUid = getExistingRelationPageUid();
if (pageUid === undefined) return [];
const r = await window.roamAlphaAPI.data.async.q(
`[:find ?ruid ?rdata :where
[?p :block/uid "${pageUid}"]
[?p :block/children ?c]
[?c :block/uid ?ruid]
[?c :block/props ?pr]
[(get ?pr :${DISCOURSE_GRAPH_PROP_NAME}) ?rdata] ]`,
);
return r.map((x) => ({
relationId: x[0] as string,
...(x[1] as ReifiedRelationData),
}));
};

export const createReifiedRelation = async ({
sourceUid,
relationBlockUid,
Expand Down
306 changes: 266 additions & 40 deletions apps/roam/src/utils/publishNodesToGroups.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,26 @@
import { CrossAppNode } from "@repo/database/crossAppContracts";
import {
CrossAppNode,
CrossAppRelation,
CrossAppRelationTripleSchema,
} from "@repo/database/crossAppContracts";
import type { DGSupabaseClient } from "@repo/database/lib/client";
import { getAvailableGroupIds } from "@repo/database/lib/groups";
import { nodeUidsWithTypeToCrossApp } from "./roamToCrossAppConverters";
import {
reifiedRelationToCrossApp,
relationTripleSchemaToCrossApp,
} from "./roamToCrossAppConverters";
import getDiscourseRelations from "./getDiscourseRelations";
import { getReifiedRelations } from "./createReifiedBlock";
import {
crossAppRelationToDbConcept,
crossAppRelationTripleSchemaToDbConcept,
} from "@repo/database/lib/crossAppConverters";
import type { LocalConceptDataInput } from "@repo/database/inputTypes";

Check warning on line 19 in apps/roam/src/utils/publishNodesToGroups.ts

View workflow job for this annotation

GitHub Actions / eslint (apps/roam)

[eslint (apps/roam)] apps/roam/src/utils/publishNodesToGroups.ts#L19 

'LocalConceptDataInput' is defined but never used           @typescript-eslint/no-unused-vars
Raw output 
   19:15  warning  'LocalConceptDataInput' is defined but never used           @typescript-eslint/no-unused-vars
import { ensureSpaceAccess } from "@repo/database/lib/groups";
import { isIgnorableUpsertError } from "@repo/database/lib/contextFunctions";
import type { TablesInsert } from "@repo/database/dbTypes";
import posthog from "posthog-js";

Check warning on line 23 in apps/roam/src/utils/publishNodesToGroups.ts

View workflow job for this annotation

GitHub Actions / eslint (apps/roam)

[eslint (apps/roam)] apps/roam/src/utils/publishNodesToGroups.ts#L23 

'posthog' is defined but never used                         @typescript-eslint/no-unused-vars
Raw output 
   23:8   warning  'posthog' is defined but never used                         @typescript-eslint/no-unused-vars

export type NodeUidWithType = {
uid: string;
Expand All @@ -10,14 +29,202 @@

type PublishNodesResult = {
publishedNodeUids: string[];
publishedRelationUids: string[];
publishedNodeSchemaUids: string[];
publishedRelationTripleSchemaUids: string[];
skippedUnsyncedUids: string[];
okGroupIds: string[];
failedGroupIds: string[];
};

// 23505 = unique_violation: the grant already exists, which counts as success.
const isIgnorableUpsertError = (error: { code?: string } | null): boolean =>
!error || error.code === "23505";
const getAllPublishedIdsByGroup = async (
client: DGSupabaseClient,
spaceId: number,
groupIds: string[],
): Promise<Record<string, Set<string>>> => {
const response = await client
.from("ResourceAccess")
.select("account_uid, source_local_id")
.eq("space_id", spaceId)
.in("account_uid", groupIds);
if (response.error) throw response.error;
const publishedIdsByGroupId = Object.fromEntries(
groupIds.map((gid) => [gid, new Set<string>()]),
);
response.data.forEach(({ account_uid, source_local_id }) => {
publishedIdsByGroupId[account_uid].add(source_local_id);
});

return publishedIdsByGroupId;
};

const getSpaceIdAndUrlsByGroupId = async (
client: DGSupabaseClient,
groupIds: string[],
): Promise<{
spaceUrlById: Record<number, string>;
spaceIdsByGroupId: Record<string, Set<number>>;
}> => {
const response = await client
.from("SpaceAccess")
.select("account_uid, space_id")
.in("account_uid", groupIds);
if (response.error) throw response.error;
const spaceIds = response.data.map((r) => r.space_id);
const response2 = await client
.from("Space")
.select("id, url")
.in("id", spaceIds);
if (response2.error) throw response2.error;
const spaceUrlById = Object.fromEntries(
response2.data.map(({ id, url }) => [id, url]),
);
const spaceIdsByGroupId = Object.fromEntries(
groupIds.map((gid) => [gid, new Set<number>()]),
);
response.data.forEach(({ account_uid, space_id }) => {
spaceIdsByGroupId[account_uid].add(space_id);
});
return {
spaceUrlById,
spaceIdsByGroupId,
};
};

// Use readImportedSourceIdentity from eng-1859 when it's merged.
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const importedFromSpaceId = (nodeId: string): number | undefined => undefined;

export const gatherCorrespondingRelations = async ({
client,
spaceId,
groupIds,
syncedUids,
forNodeIds,
}: {
client: DGSupabaseClient;
spaceId: number;
groupIds: string[];
syncedUids: Set<string>;
forNodeIds?: Set<string>;
}): Promise<{
relations: CrossAppRelation[];
relationTripleSchemas: CrossAppRelationTripleSchema[];
publishInfo: TablesInsert<"ResourceAccess">[];
}> => {
const allRelationsSchemas = getDiscourseRelations();
const allRelationSchemasById = Object.fromEntries(
allRelationsSchemas.map((s) => [s.id, s]),
);
// Should we even handle non-reified relations? Assuming not.
// I need a way to know if a relation is imported, see importedFromSpaceId
const allRelations = await getReifiedRelations();
const spaceIdOfNodes: Record<string, number> = {};
const isImportedFrom = (nodeLocalId: string): number => {
let cached = spaceIdOfNodes[nodeLocalId];
if (cached === undefined) {
cached = spaceIdOfNodes[nodeLocalId] =
importedFromSpaceId(nodeLocalId) || spaceId;
}
return cached === spaceId ? 0 : cached;
};
Comment on lines +123 to +130

@graphite-app graphite-app Bot Jul 15, 2026

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.

Critical Logic Bug: The isImportedFrom function always returns 0 because getSpaceIdOf (line 67) always returns undefined.

// Line 259: getSpaceIdOf always returns undefined
cached = sourceIdOfNodes[nodeLocalId] = undefined || spaceId;
// So cached always equals spaceId

// Line 261: Since cached === spaceId is always true
return cached === spaceId ? 0 : cached;  // Always returns 0

This breaks the logic at lines 279-281 where (isImportedFrom(r.sourceUid) || 0) in groupSpaces evaluates to 0 in groupSpaces, checking if property "0" exists instead of the actual space ID. This will cause relations with imported nodes to be incorrectly filtered.

Similarly at lines 102-104, spaceUids[isImportedFrom(r.sourceUid) || 0] always looks up spaceUids[0] which likely doesn't exist, causing all imported nodes to fall back to local UIDs when they should use RIDs.

Impact: Relations involving imported nodes will not be published correctly to groups. The code will either skip valid relations or fail to construct proper RIDs for cross-space references.

Fix: Either implement getSpaceIdOf properly or remove the imported node logic until ENG-1856 is complete (as mentioned in the PR description).

Spotted by Graphite

Fix in Graphite

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

@maparent maparent Jul 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is valid, but it's related to function that is expected to be integrated in ENG-1865 (forthcoming.) So the bug is really a placeholder.

const relations =
forNodeIds !== undefined
? allRelations.filter(
(r) =>
r.importedFromRid === undefined &&
(forNodeIds.has(r.sourceUid) || forNodeIds.has(r.destinationUid)),
)
: allRelations.filter((r) => r.importedFromRid === undefined);
const { spaceIdsByGroupId, spaceUrlById } = await getSpaceIdAndUrlsByGroupId(
client,
groupIds,
);
const isImportedFromSpaceUri = (uid: string) =>
spaceUrlById[isImportedFrom(uid) || 0];
const publishedIdsByGroup = await getAllPublishedIdsByGroup(
client,
spaceId,
groupIds,
);
// calculate separately to avoid case of a relation between nodes published to or from different groups
const relevantRelationIdsPerGroupId = Object.fromEntries(
groupIds.map((groupId) => {
const groupSpaceIds = spaceIdsByGroupId[groupId];
const publishedIds = publishedIdsByGroup[groupId];
return [
groupId,
relations
.filter(
(r) =>
(publishedIds.has(r.sourceUid) ||
groupSpaceIds.has(isImportedFrom(r.sourceUid) || 0)) &&
(publishedIds.has(r.destinationUid) ||
groupSpaceIds.has(isImportedFrom(r.destinationUid) || 0)),
)
.map((r) => r.relationId),
];
}),
);
const allRelevantRelationIds = new Set(
Object.values(relevantRelationIdsPerGroupId).flat(),
);
let allRelevantRelations = relations.filter((r) =>
allRelevantRelationIds.has(r.relationId),
);
const relationSchemaIds = new Set(
allRelevantRelations
.map((r) => r.hasSchema)
// filter out deleted schemas
.filter((id) => id in allRelationSchemasById),
);
allRelevantRelations = allRelevantRelations.filter((r) =>
relationSchemaIds.has(r.hasSchema),
);
const missingRelationSchemaTriples = allRelationsSchemas.filter(
(r) => relationSchemaIds.has(r.id) && !syncedUids.has(r.id),
);
const missingRelations = allRelevantRelations.filter(
(r) => !syncedUids.has(r.relationId),
);

const publishInfo = [];
for (const groupId of groupIds) {
const groupRelationIds = new Set(relevantRelationIdsPerGroupId[groupId]);
const groupMissingRelations = missingRelations.filter((r) =>
groupRelationIds.has(r.relationId),
);
const groupMissingRelationIds = groupMissingRelations.map(
(r) => r.relationId,
);
const groupSchemaIds = new Set(
groupMissingRelations.map((r) => r.hasSchema),
);
const groupMissingSchemaIds = missingRelationSchemaTriples
.filter((rs3) => groupSchemaIds.has(rs3.id))
.map((rs3) => rs3.id);
const groupMissingIds = [
...groupMissingRelationIds,
...groupMissingSchemaIds,
];
publishInfo.push(
...groupMissingIds.map((sourceLocalId) => ({
account_uid: groupId,
source_local_id: sourceLocalId,
space_id: spaceId,
})),
);
}
return {
relations: missingRelations
.map((r) => reifiedRelationToCrossApp(r, isImportedFromSpaceUri))
.filter((r) => r !== null),
relationTripleSchemas: missingRelationSchemaTriples
.map((rs3) => relationTripleSchemaToCrossApp(rs3))
.filter((rs3) => rs3 !== null),
publishInfo,
};
};

const onlyStrings = (values: (string | null)[]): string[] =>
values.filter((value): value is string => typeof value === "string");
Expand All @@ -43,6 +250,9 @@
}): Promise<PublishNodesResult> => {
const result: PublishNodesResult = {
publishedNodeUids: [],
publishedRelationUids: [],
publishedNodeSchemaUids: [],
publishedRelationTripleSchemaUids: [],
skippedUnsyncedUids: [],
okGroupIds: [],
failedGroupIds: [],
Expand All @@ -59,57 +269,45 @@
);
if (targetGroupIds.length === 0) return result;

const uids = [...new Set(nodes.map((node) => node.localId))];
const nodeUids = [...new Set(nodes.map((node) => node.localId))];

const syncedRes = await client
.from("my_concepts")
.select("source_local_id")
.eq("space_id", spaceId)
.eq("is_schema", false)
.in("source_local_id", uids);
.eq("space_id", spaceId);
if (syncedRes.error) throw syncedRes.error;
const syncedUids = new Set(
onlyStrings((syncedRes.data ?? []).map((row) => row.source_local_id)),
);

result.skippedUnsyncedUids = uids.filter((uid) => !syncedUids.has(uid));
const syncedNodeUids = uids.filter((uid) => syncedUids.has(uid));
result.skippedUnsyncedUids = nodeUids.filter((uid) => !syncedUids.has(uid));
const syncedNodeUids = nodeUids.filter((uid) => syncedUids.has(uid));
if (syncedNodeUids.length === 0) return result;

// Required dependency: the node-type schema concept, when it is synced too.
const types = [
...new Set(
nodes
.filter((node) => syncedUids.has(node.localId))
.map((node) => node.nodeType),
),
];
const schemaRes = await client
.from("my_concepts")
.select("source_local_id")
.eq("space_id", spaceId)
.eq("is_schema", true)
.in("source_local_id", types);
if (schemaRes.error) throw schemaRes.error;
const syncedSchemaIds = onlyStrings(
(schemaRes.data ?? []).map((row) => row.source_local_id),
const nodeSchemaUids = new Set(nodes.map((node) => node.nodeType));
const missingNodeSchemaUids = [...nodeSchemaUids].filter(

Check warning on line 288 in apps/roam/src/utils/publishNodesToGroups.ts

View workflow job for this annotation

GitHub Actions / eslint (apps/roam)

[eslint (apps/roam)] apps/roam/src/utils/publishNodesToGroups.ts#L288 

'missingNodeSchemaUids' is assigned a value but never used  @typescript-eslint/no-unused-vars
Raw output 
  288:9   warning  'missingNodeSchemaUids' is assigned a value but never used  @typescript-eslint/no-unused-vars
(uid) => !syncedUids.has(uid),
);
const syncedNodeSchemaUids = [...nodeSchemaUids].filter((uid) =>
syncedUids.has(uid),
);

const resourceIds = [...syncedNodeUids, ...syncedSchemaIds];
const resourceIds = [...syncedNodeUids, ...syncedNodeSchemaUids];

for (const groupId of targetGroupIds) {
// Existing reader/editor access is broader than partial, so leave it intact.
const spaceAccessRes = await client
.from("SpaceAccess")
.upsert(
{ account_uid: groupId, space_id: spaceId, permissions: "partial" },
{ ignoreDuplicates: true },
);
if (!isIgnorableUpsertError(spaceAccessRes.error)) {
result.failedGroupIds.push(groupId);
continue;
}
const { existing: existingSpaceAccess, missing: missingSpaceAccess } =
await ensureSpaceAccess({
client,
groupIds,
spaceId,
});
if (missingSpaceAccess && Object.keys(missingSpaceAccess).length) {
result.failedGroupIds = groupIds.filter(
(id) => !(id in existingSpaceAccess),
);
groupIds = Object.keys(existingSpaceAccess);
}

for (const groupId of targetGroupIds) {
const grantRes = await client.from("ResourceAccess").upsert(
resourceIds.map((sourceLocalId) => ({
account_uid: groupId,
Expand All @@ -125,6 +323,34 @@

result.okGroupIds.push(groupId);
}
const { relations, relationTripleSchemas, publishInfo } =
await gatherCorrespondingRelations({
client,
spaceId,
groupIds: targetGroupIds,
syncedUids,
forNodeIds: new Set(syncedNodeUids),
});
const upsertConcepts = [
...relationTripleSchemas.map((rs3) =>
crossAppRelationTripleSchemaToDbConcept(rs3),
),
...relations.map((r) => crossAppRelationToDbConcept(r)),
].filter((r) => r !== undefined);

const response = await client.rpc("upsert_concepts", {
v_space_id: spaceId,
data: upsertConcepts,
});
if (response.error) throw response.error;
result.publishedRelationTripleSchemaUids = relationTripleSchemas.map(
(s) => s.localId,
);
result.publishedRelationUids = relations.map((s) => s.localId);
const grantRes = await client
.from("ResourceAccess")
.upsert(publishInfo, { ignoreDuplicates: true });
if (!isIgnorableUpsertError(grantRes.error)) throw grantRes.error;

result.publishedNodeUids = result.okGroupIds.length > 0 ? syncedNodeUids : [];
return result;
Expand Down
Loading
Loading