Skip to content
Draft
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: 2 additions & 0 deletions apps/obsidian/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ for (let leaf of app.workspace.getActiveLeavesOfType(MY_VIEW_TYPE)) {

### Mobile compatibility

- The plugin ships with `isDesktopOnly: false`, so it must load on mobile
- Node builtins and `electron` are deliberately not listed in the esbuild `external` array in `scripts/compile.ts`. Adding them back would let a dependency's `require("fs")` survive into the bundle and throw at runtime on mobile; leaving them out makes the build fail instead. Replace the dependency rather than re-adding the external
- Node.js and Electron APIs (`fs`, `crypto`, `os`) are unavailable on mobile
- If the plugin targets mobile, use web API equivalents: `SubtleCrypto` instead of `crypto`, `navigator.clipboard` for clipboard access
- Regex lookbehind assertions are not supported on some mobile — avoid them if possible
Expand Down
2 changes: 1 addition & 1 deletion apps/obsidian/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@
"description": "Add semantic structure to your notes with the Discourse Graph protocol.",
"author": "Discourse Graphs",
"authorUrl": "https://discoursegraphs.com",
"isDesktopOnly": true
"isDesktopOnly": false
}
11 changes: 5 additions & 6 deletions apps/obsidian/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
"lint": "eslint .",
"lint:fix": "eslint . --fix",
"publish": "tsx scripts/publish.ts",
"check-types": "tsc --noEmit --skipLibCheck"
"check-types": "tsc --noEmit --skipLibCheck",
"test": "pnpm test:unit",
"test:unit": "vitest run --config vitest.config.mts"
},
"keywords": [],
"author": "",
Expand All @@ -19,12 +21,10 @@
"@octokit/core": "^6.1.2",
"@repo/eslint-config": "workspace:*",
"@repo/typescript-config": "workspace:*",
"@types/mime-types": "3.0.1",
"@types/node": "catalog:",
"@types/react": "catalog:obsidian",
"@types/react-dom": "catalog:obsidian",
"autoprefixer": "^10.4.21",
"builtin-modules": "3.3.0",
"dotenv": "^16.4.5",
"esbuild": "0.17.3",
"eslint": "catalog:",
Expand All @@ -35,16 +35,15 @@
"tsx": "^4.19.2",
"typescript": "5.5.4",
"uuidv7": "1.1.0",
"zod": "^3.24.1"
"zod": "^3.24.1",
"vitest": "catalog:"
},
"dependencies": {
"@codemirror/view": "^6.38.8",
"@repo/database": "workspace:*",
"@repo/utils": "workspace:*",
"@supabase/supabase-js": "catalog:",
"date-fns": "^4.1.0",
"gray-matter": "^4.0.3",
"mime-types": "^3.0.1",
"nanoid": "^4.0.2",
"react": "catalog:obsidian",
"react-dom": "catalog:obsidian",
Expand Down
8 changes: 5 additions & 3 deletions apps/obsidian/scripts/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import esbuild from "esbuild";
import fs from "fs";
import path from "path";
import { z } from "zod";
import builtins from "builtin-modules";
import dotenv from "dotenv";
import postcss from "postcss";
import tailwindcss from "tailwindcss";
Expand Down Expand Up @@ -43,9 +42,12 @@ export const args = {
format: "cjs",
root: ".",
mirror: process.env.OBSIDIAN_PLUGIN_PATH,
// Node builtins and "electron" are deliberately NOT external. Marking them
// external lets a dependency's `require("fs")` survive into the bundle, which
// throws at runtime on Obsidian mobile. Leaving them out makes esbuild fail
// the build instead, so a mobile-breaking dependency cannot land unnoticed.
external: [
"obsidian",
"electron",
"@codemirror/autocomplete",
"@codemirror/collab",
"@codemirror/commands",
Expand All @@ -58,7 +60,6 @@ export const args = {
"@lezer/highlight",
"@lezer/lr",
"tslib=window.TSLib",
...builtins,
],
} as CliOpts;

Expand Down Expand Up @@ -115,6 +116,7 @@ export const compile = ({
outdir,
bundle: true,
format,
platform: "browser",
sourcemap: isProd ? undefined : "inline",
minify: isProd,
entryNames: out,
Expand Down
51 changes: 51 additions & 0 deletions apps/obsidian/scripts/publish.ts
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,55 @@ const updateManifest = (tempDir: string, version: string): void => {

fs.writeFileSync(manifestPath, JSON.stringify(manifest, null, 2));
log(`Updated manifest version to ${version}`);
log(` isDesktopOnly: ${manifest.isDesktopOnly}`);
};

// Everything published is read from the current working directory, so running
// this from the wrong checkout silently ships that checkout's manifest. Printing
// the branch alongside the flags that change who can install the plugin makes
// that mistake visible before anything is pushed.
const logReleaseProvenance = async (obsidianDir: string): Promise<void> => {
const manifestPath = path.join(obsidianDir, "manifest.json");
if (!fs.existsSync(manifestPath)) {
throw new Error(`manifest.json not found in ${obsidianDir}`);
}
const manifest = JSON.parse(fs.readFileSync(manifestPath, "utf8"));

let branch = "unknown";
try {
const { stdout } = await execPromise("git rev-parse --abbrev-ref HEAD", {
cwd: obsidianDir,
});
branch = stdout.trim();
} catch {
// A detached HEAD or a missing git dir should not block a publish
}

log("Publishing from:");
log(` directory: ${obsidianDir}`);
log(` branch: ${branch}`);
log(` isDesktopOnly: ${manifest.isDesktopOnly}`);
log(` minAppVersion: ${manifest.minAppVersion}`);
};

// The mirror-repo copy takes manifest.json from dist/ while the release assets
// take it from source, so a stale dist/ would publish two different manifests.
const assertBuiltManifestMatchesSource = (obsidianDir: string): void => {
const sourcePath = path.join(obsidianDir, "manifest.json");
const builtPath = path.join(obsidianDir, "dist", "manifest.json");
if (!fs.existsSync(builtPath)) {
throw new Error("dist/manifest.json not found — build the plugin first");
}

const source = fs.readFileSync(sourcePath, "utf8").trim();
const built = fs.readFileSync(builtPath, "utf8").trim();
if (source !== built) {
throw new Error(
"manifest.json and dist/manifest.json disagree. The build is stale, so " +
"the mirrored repo and the release assets would ship different " +
"manifests. Rebuild before publishing.",
);
}
};

const copyBuildFiles = (buildDir: string, tempDir: string): void => {
Expand Down Expand Up @@ -739,6 +788,8 @@ const publish = async (config: PublishConfig): Promise<void> => {
log(`Publishing Obsidian plugin v${version} (${releaseType} release)`);

await buildPlugin(obsidianDir);
assertBuiltManifestMatchesSource(obsidianDir);
await logReleaseProvenance(obsidianDir);

if (fs.existsSync(tempDir)) {
fs.rmSync(tempDir, { recursive: true });
Expand Down
10 changes: 6 additions & 4 deletions apps/obsidian/src/components/NodeSearchModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -516,8 +516,8 @@ const NodeSearch = ({
sortKey={sortKey}
/>
</div>
<div className="border-modifier-border mt-3 flex flex-1 overflow-hidden rounded border">
<div className="border-modifier-border flex w-2/5 flex-col border-r">
<div className="border-modifier-border mt-3 flex flex-1 flex-col overflow-hidden rounded border sm:flex-row">
<div className="border-modifier-border flex min-h-0 flex-1 flex-col border-b sm:w-2/5 sm:flex-none sm:border-b-0 sm:border-r">
{candidateState.status === "loading" && (
<div className="text-muted p-4">Loading discourse nodes…</div>
)}
Expand Down Expand Up @@ -566,12 +566,14 @@ export class NodeSearchModal extends Modal {
onOpen() {
const { contentEl, modalEl } = this;
// The default modal is too narrow for a result list beside a preview pane.
// Responsive layout is an explicit non-goal, so this is a desktop-only size.
// Below `sm` the two panes stack instead, so the width is only claimed once
// there is room for the side-by-side layout.
modalEl.addClasses([
"dg-node-search-modal",
"h-[600px]",
"max-h-[80vh]",
"w-[900px]",
"w-full",
"sm:w-[900px]",
"max-w-[90vw]",
]);
contentEl.addClasses(["flex", "h-full", "flex-col", "overflow-hidden"]);
Expand Down
36 changes: 36 additions & 0 deletions apps/obsidian/src/utils/__tests__/mimeType.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { describe, expect, it } from "vitest";
import { DEFAULT_MIME_TYPE, getMimeTypeForPath } from "~/utils/mimeType";

describe("getMimeTypeForPath", () => {
it("resolves common attachment types", () => {
expect(getMimeTypeForPath("assets/diagram.png")).toBe("image/png");
expect(getMimeTypeForPath("notes/paper.pdf")).toBe("application/pdf");
expect(getMimeTypeForPath("clip.mp4")).toBe("video/mp4");
});

it("is case insensitive", () => {
expect(getMimeTypeForPath("Photo.JPG")).toBe("image/jpeg");
});

it("prefixes text formats with text/ so callers can skip them", () => {
expect(getMimeTypeForPath("note.md").startsWith("text/")).toBe(true);
expect(getMimeTypeForPath("data.csv").startsWith("text/")).toBe(true);
});

it("uses the last extension of a multi-dot name", () => {
expect(getMimeTypeForPath("archive.tar.png")).toBe("image/png");
});

it("falls back for unknown and extensionless paths", () => {
expect(getMimeTypeForPath("notes/README")).toBe(DEFAULT_MIME_TYPE);
expect(getMimeTypeForPath("thing.unknownext")).toBe(DEFAULT_MIME_TYPE);
});

it("does not treat a dotfile as an extension", () => {
expect(getMimeTypeForPath(".gitignore")).toBe(DEFAULT_MIME_TYPE);
});

it("ignores dots in parent directories", () => {
expect(getMimeTypeForPath("my.folder/file")).toBe(DEFAULT_MIME_TYPE);
});
});
58 changes: 58 additions & 0 deletions apps/obsidian/src/utils/__tests__/splitFrontmatter.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import { describe, expect, it } from "vitest";
import { splitFrontmatter } from "~/utils/splitFrontmatter";

describe("splitFrontmatter", () => {
it("splits a standard block", () => {
expect(splitFrontmatter("---\na: 1\nb: two\n---\nbody here")).toEqual({
yaml: "a: 1\nb: two",
body: "body here",
});
});

it("handles CRLF line endings", () => {
expect(splitFrontmatter("---\r\na: 1\r\n---\r\nbody")).toEqual({
yaml: "a: 1",
body: "body",
});
});

it("treats an empty block as present but empty", () => {
expect(splitFrontmatter("---\n---\nbody")).toEqual({
yaml: "",
body: "body",
});
});

it("returns no frontmatter when the file does not start with a fence", () => {
const content = "intro text\n---\na: 1\n---\nbody";
expect(splitFrontmatter(content)).toEqual({ yaml: null, body: content });
});

it("leaves a horizontal rule in the body alone", () => {
expect(splitFrontmatter("---\na: 1\n---\nbody\n---\nafter rule")).toEqual({
yaml: "a: 1",
body: "body\n---\nafter rule",
});
});

it("handles a file that is only frontmatter", () => {
expect(splitFrontmatter("---\na: 1\n---")).toEqual({
yaml: "a: 1",
body: "",
});
});

it("returns no frontmatter for content with no fence at all", () => {
expect(splitFrontmatter("just a note")).toEqual({
yaml: null,
body: "just a note",
});
});

it("keeps multi-line YAML values intact", () => {
expect(splitFrontmatter("---\ntags:\n - a\n - b\n---\nbody")).toEqual({
yaml: "tags:\n - a\n - b",
body: "body",
});
});
});
34 changes: 22 additions & 12 deletions apps/obsidian/src/utils/importNodes.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import type { Json } from "@repo/database/dbTypes";
import matter from "gray-matter";
import { App, Notice, TFile } from "obsidian";
import { App, Notice, TFile, parseYaml } from "obsidian";
import type { DGSupabaseClient } from "@repo/database/lib/client";
import { listGroupSharedNodes } from "@repo/database/lib/sharedNodes";
import type DiscourseGraphPlugin from "~/index";
import { getLoggedInClient, getSupabaseContext } from "./supabaseContext";
import type { DiscourseNode, ImportableNode } from "~/types";
import { QueryEngine } from "~/services/QueryEngine";
import { splitFrontmatter } from "~/utils/splitFrontmatter";
import {
getImportedNodesInfo,
getLocalNodeKeyToEndpointId,
Expand Down Expand Up @@ -725,13 +725,18 @@ const updateMarkdownAssetLinks = ({
},
);

// Match markdown links (non-image): [text](path) — internal paths resolved like wikilinks, href kept URL-encoded
const markdownLinkRegex = /(?<!!)\[([^\]]*)\]\(([^)]+)\)/g;
// Match markdown links (non-image): [text](path) — internal paths resolved like wikilinks, href kept URL-encoded.
// The leading `!` is captured rather than excluded with a lookbehind, which
// older mobile WebViews do not support.
const markdownLinkRegex = /(!?)\[([^\]]*)\]\(([^)]+)\)/g;
updatedContent = updatedContent.replace(
markdownLinkRegex,
(match, linkText: string, linkPath: string) => {
if (!linkPath) return match;
linkPath = linkPath
(match, ...groups: string[]) => {
const [imagePrefix, linkText, rawLinkPath] = groups;
// An `!` prefix makes this an image embed, handled by the next pass
if (imagePrefix) return match;
if (!rawLinkPath) return match;
const linkPath = rawLinkPath
.split("/")
.map((segment) => {
try {
Expand Down Expand Up @@ -1028,14 +1033,19 @@ type ParsedFrontmatter = {
[key: string]: unknown;
};

// Unparseable frontmatter is treated as absent rather than thrown, matching how
// Obsidian itself tolerates bad YAML. `gray-matter` threw here instead.
const parseFrontmatter = (
content: string,
): { frontmatter: ParsedFrontmatter; body: string } => {
const { data, content: body } = matter(content);
return {
frontmatter: (data ?? {}) as ParsedFrontmatter,
body: body ?? "",
};
const { yaml, body } = splitFrontmatter(content);
if (yaml === null) return { frontmatter: {}, body };
try {
const data = parseYaml(yaml) as ParsedFrontmatter | null;
return { frontmatter: data ?? {}, body };
} catch {
return { frontmatter: {}, body: content };
Comment on lines +1046 to +1047

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.

When YAML parsing fails, the function returns body: content (the original full content), but this is inconsistent with the successful parse path. When splitFrontmatter succeeds but parseYaml throws, the frontmatter block markers (---) are already stripped from body, so returning the original content re-introduces them.

Impact: A file with malformed YAML frontmatter will have the raw frontmatter block (including --- delimiters) left in the body text, which will render as markdown content instead of being hidden.

Fix: Return the stripped body instead:

catch {
  return { frontmatter: {}, body };
}

This way, the malformed frontmatter block is removed from the body (matching the successful parse behavior), and users get empty frontmatter instead of a parsing error (the desired tolerance behavior mentioned in the PR description).

Suggested change
} catch {
return { frontmatter: {}, body: content };
} catch {
return { frontmatter: {}, body };

Spotted by Graphite

Fix in Graphite


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

}
};

/**
Expand Down
Loading