Skip to content

Commit 16cf49c

Browse files
author
merge-queue-bot
committed
Merge PR #513: fix(obsidian): make cross-file resolution work in the WASM engine
2 parents 47640fb + 9e85628 commit 16cf49c

10 files changed

Lines changed: 330 additions & 20 deletions

File tree

editors/obsidian/src/main.test.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,16 +41,23 @@ function makeApp(): {
4141
};
4242
workspace: {
4343
on(event: string, cb: () => unknown): unknown;
44+
onLayoutReady(cb: () => unknown): void;
4445
getActiveViewOfType(_t: unknown): FakeView | null;
4546
getLeavesOfType(_t: string): unknown[];
4647
};
4748
};
4849
active: { view: FakeView | null };
4950
fireModify(path: string): void;
51+
fireLayoutReady(): void;
52+
layoutReadyCount(): number;
5053
modifyListenerCount(): number;
5154
} {
5255
type Handler = (f: FakeFile) => unknown;
5356
const vaultHandlers: Record<string, Handler[]> = {};
57+
// Captured onLayoutReady callbacks. The fake never runs them eagerly,
58+
// modelling Obsidian's cold start where the vault index is not ready
59+
// when onload runs; fireLayoutReady() simulates the vault becoming ready.
60+
const layoutReadyCbs: Array<() => unknown> = [];
5461
const active: { view: FakeView | null } = { view: null };
5562
return {
5663
app: {
@@ -71,6 +78,9 @@ function makeApp(): {
7178
on(_event: string, _cb: () => unknown): unknown {
7279
return {};
7380
},
81+
onLayoutReady(cb: () => unknown): void {
82+
layoutReadyCbs.push(cb);
83+
},
7484
getActiveViewOfType(_t: unknown): FakeView | null {
7585
return active.view;
7686
},
@@ -83,6 +93,12 @@ function makeApp(): {
8393
fireModify(path: string): void {
8494
for (const cb of vaultHandlers["modify"] ?? []) cb({ path });
8595
},
96+
fireLayoutReady(): void {
97+
for (const cb of layoutReadyCbs) cb();
98+
},
99+
layoutReadyCount(): number {
100+
return layoutReadyCbs.length;
101+
},
86102
modifyListenerCount(): number {
87103
return vaultHandlers["modify"]?.length ?? 0;
88104
},
@@ -375,3 +391,30 @@ describe("engine-down / restart safety (Copilot review)", () => {
375391
expect(ok).toBe(false);
376392
});
377393
});
394+
395+
describe("onload — defers the first snapshot to layout-ready (cold-start race)", () => {
396+
// On a cold Obsidian start the vault file index is not fully populated
397+
// when onload runs, so app.vault.getMarkdownFiles() — the workspace-
398+
// snapshot source — can return a partial list, dropping deep
399+
// include/link targets and making cross-file directives report a
400+
// missing file. onload must defer the first startRuntime() until
401+
// app.workspace.onLayoutReady fires, when the vault is fully indexed.
402+
test("startRuntime runs on layout-ready, not during onload", async () => {
403+
const { plugin, harness } = makePlugin();
404+
const startSpy = mock(() => Promise.resolve(true));
405+
(
406+
plugin as unknown as { startRuntime: () => Promise<boolean> }
407+
).startRuntime = startSpy;
408+
409+
await plugin.onload();
410+
411+
// Deferred: onload registered exactly one layout-ready callback and
412+
// has NOT snapshotted the vault yet.
413+
expect(startSpy).not.toHaveBeenCalled();
414+
expect(harness.layoutReadyCount()).toBe(1);
415+
416+
// The vault finishes loading: the snapshot runs now.
417+
harness.fireLayoutReady();
418+
expect(startSpy).toHaveBeenCalledTimes(1);
419+
});
420+
});

editors/obsidian/src/main.ts

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,14 @@
77
// workspace, diagnostics, actions, settings, wiring); the methods here
88
// just orchestrate the Obsidian lifecycle.
99
//
10-
// onload order (plan 217 §Lifecycle): read settings → load the WASM
11-
// bundle → build the workspace snapshot → createRuntime once → register
12-
// the CM6 extension, commands, diagnostics view, and vault listeners.
10+
// onload order (plan 217 §Lifecycle): read settings → register the CM6
11+
// extension, commands, diagnostics view, and vault listeners → then,
12+
// once app.workspace.onLayoutReady fires, build the workspace snapshot
13+
// and createRuntime. The snapshot is deferred to layout-ready because
14+
// app.vault.getMarkdownFiles() is only complete after the vault finishes
15+
// indexing; running it during a cold-start onload can return a partial
16+
// file list, which drops deep include/link targets and makes cross-file
17+
// directives report a missing file.
1318
// onunload disposes the runtime, cancels listeners, and detaches the
1419
// view. A "Restart session" command runs the same dispose + create
1520
// flow a configPath change uses.
@@ -110,7 +115,12 @@ export default class MdsmithPlugin extends Plugin {
110115
this.registerActiveFileCheck();
111116
this.registerCursorCommands();
112117

113-
await this.startRuntime();
118+
// Defer the first snapshot until the vault index is fully populated.
119+
// onLayoutReady runs the callback once the vault is ready — or
120+
// immediately if it already is (e.g. the plugin is enabled after
121+
// startup) — so app.vault.getMarkdownFiles() returns the complete
122+
// file list and cross-file include/link resolution sees every file.
123+
this.app.workspace.onLayoutReady(() => void this.startRuntime());
114124
}
115125

116126
override onunload(): void {

editors/obsidian/src/plugin.e2e.test.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,10 @@ async function bootPlugin(
243243
on(_event: string, _cb: () => unknown): unknown {
244244
return {};
245245
},
246+
// onload defers the first snapshot to onLayoutReady; this e2e boots
247+
// the runtime explicitly below (and awaits it), so the fake captures
248+
// the callback without firing it to avoid starting the runtime twice.
249+
onLayoutReady(_cb: () => unknown): void {},
246250
getActiveViewOfType(_t: unknown): FakeMarkdownView | null {
247251
return active.view;
248252
},
@@ -281,6 +285,12 @@ async function bootPlugin(
281285
internals.saveData = async () => {};
282286

283287
await plugin.onload();
288+
// onload now defers startRuntime() to onLayoutReady (faked as a no-op
289+
// above); start the runtime here and await it so the assertions below
290+
// run against a ready engine, exactly as the pre-deferral onload did.
291+
await (
292+
plugin as unknown as { startRuntime(): Promise<boolean> }
293+
).startRuntime();
284294

285295
return {
286296
plugin,

internal/engine/runner.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -553,6 +553,15 @@ func (r *Runner) populateFileFields(f *lint.File, path string) {
553553
case filepath.IsAbs(path):
554554
gitignoreDir = filepath.Dir(path)
555555
}
556+
// An in-memory workspace (the Session's MemWorkspace, e.g. the WASM
557+
// build) has no on-disk RootDir, but its SourceFS is rooted at the
558+
// project root — the same contract os.DirFS(RootDir) gives on disk.
559+
// Wire it as RootFS so RootFS-aware cross-file rules (include's ".."
560+
// resolution, MDS020's schema reads) read through the workspace
561+
// instead of falling back to os.*, which is "not implemented on js".
562+
if r.RootDir == "" && r.SourceFS != nil {
563+
f.RootFS = r.SourceFS
564+
}
556565
if gitignoreDir != "" {
557566
gd := gitignoreDir
558567
f.GitignoreFunc = func() *gitignore.Matcher {

internal/rules/crossfilereferenceintegrity/rule.go

Lines changed: 58 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"io/fs"
66
"net/url"
77
"os"
8+
"path"
89
"path/filepath"
910
"strings"
1011

@@ -522,12 +523,24 @@ func (r *Rule) checkRelativeTarget(
522523
// resolveTargetFile when only file existence (not the read
523524
// helper) matters.
524525
func targetExists(f *lint.File, linkPath, resolvedRoot string) bool {
525-
if path, ok := resolveTargetOSPath(f.Path, linkPath); ok && cachedStatExists(path) {
526-
if resolvedRoot == "" || isWithinRoot(resolvedRoot, path) {
526+
if osPath, ok := resolveTargetOSPath(f.Path, linkPath); ok && cachedStatExists(osPath) {
527+
if resolvedRoot == "" || isWithinRoot(resolvedRoot, osPath) {
527528
return true
528529
}
529530
return false
530531
}
532+
// In-memory / workspace-relative resolution: the WASM and LSP engines
533+
// have no OS disk for the branch above. Resolve the link against the
534+
// source file's directory within the project-root FS, collapsing ".."
535+
// so io/fs — which rejects paths containing ".." — can stat an
536+
// up-and-over target (e.g. docs/x.md -> ../../internal/y.md).
537+
if f.RootFS != nil && !filepath.IsAbs(f.Path) {
538+
if rel, ok := resolveWorkspaceRelTarget(f.Path, linkPath); ok {
539+
if _, err := fs.Stat(f.RootFS, rel); err == nil {
540+
return true
541+
}
542+
}
543+
}
531544
fsPath := filepath.ToSlash(linkPath)
532545
fsPath = strings.TrimPrefix(fsPath, "./")
533546
if fsPath == "" || strings.HasPrefix(fsPath, "/") {
@@ -843,23 +856,40 @@ func buildAnchorsForTarget(target targetFile) (map[string]struct{}, error) {
843856

844857
func resolveTargetFile(f *lint.File, linkPath, resolvedRoot string) (targetFile, bool) {
845858
maxBytes := f.MaxInputBytes
846-
if path, ok := resolveTargetOSPath(f.Path, linkPath); ok {
847-
if cachedStatExists(path) {
859+
if osPath, ok := resolveTargetOSPath(f.Path, linkPath); ok {
860+
if cachedStatExists(osPath) {
848861
// Reject links that resolve outside the project root,
849862
// evaluating symlinks to prevent bypass via symlinked dirs.
850-
if resolvedRoot != "" && !isWithinRoot(resolvedRoot, path) {
863+
if resolvedRoot != "" && !isWithinRoot(resolvedRoot, osPath) {
851864
return targetFile{}, false
852865
}
853866
return targetFile{
854-
cacheKey: "os:" + path,
855-
runCacheKey: path,
867+
cacheKey: "os:" + osPath,
868+
runCacheKey: osPath,
856869
read: func() ([]byte, error) {
857-
return bytelimit.ReadFileLimited(path, maxBytes)
870+
return bytelimit.ReadFileLimited(osPath, maxBytes)
858871
},
859872
}, true
860873
}
861874
}
862875

876+
// In-memory / workspace-relative resolution (see targetExists): resolve
877+
// the link within the project-root FS so an up-and-over ".." target,
878+
// which io/fs rejects as a raw path, still reads on the WASM/LSP engines.
879+
if f.RootFS != nil && !filepath.IsAbs(f.Path) {
880+
if rel, ok := resolveWorkspaceRelTarget(f.Path, linkPath); ok {
881+
if _, err := fs.Stat(f.RootFS, rel); err == nil {
882+
rootFS := f.RootFS
883+
return targetFile{
884+
cacheKey: "fs:" + rel,
885+
read: func() ([]byte, error) {
886+
return bytelimit.ReadFSFileLimited(rootFS, rel, maxBytes)
887+
},
888+
}, true
889+
}
890+
}
891+
}
892+
863893
fsPath := filepath.ToSlash(linkPath)
864894
fsPath = strings.TrimPrefix(fsPath, "./")
865895
if fsPath == "" || strings.HasPrefix(fsPath, "/") {
@@ -936,6 +966,26 @@ func resolveTargetOSPath(sourcePath, linkPath string) (string, bool) {
936966
return filepath.Clean(filepath.Join(filepath.Dir(sourcePath), linkPath)), true
937967
}
938968

969+
// resolveWorkspaceRelTarget maps a workspace-relative source path and a
970+
// file-relative link to a slash path valid for fs.Stat against the
971+
// project-root FS (f.RootFS). It joins the link onto the source file's
972+
// directory and cleans ".." away — io/fs rejects any path containing
973+
// ".." — and returns ("", false) when the result escapes the workspace
974+
// root, is empty, or is absolute, none of which name a file inside the
975+
// in-memory workspace.
976+
func resolveWorkspaceRelTarget(sourcePath, linkPath string) (string, bool) {
977+
lp := filepath.ToSlash(linkPath)
978+
if lp == "" || strings.HasPrefix(lp, "/") {
979+
return "", false
980+
}
981+
dir := path.Dir(filepath.ToSlash(sourcePath))
982+
rel := path.Clean(path.Join(dir, lp))
983+
if rel == "." || rel == ".." || strings.HasPrefix(rel, "../") {
984+
return "", false
985+
}
986+
return rel, true
987+
}
988+
939989
func isMarkdownPath(path string) bool {
940990
ext := strings.ToLower(filepath.Ext(path))
941991
return ext == ".md" || ext == ".markdown"

internal/rules/crossfilereferenceintegrity/rule_test.go

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -683,6 +683,73 @@ func TestResolveTargetFile_EmptyFSPathReturnsNotFound(t *testing.T) {
683683
require.False(t, ok, "fsPath='' after TrimPrefix must return not found")
684684
}
685685

686+
// TestTargetExists_ParentTraversalViaRootFS mirrors the WASM/Session
687+
// shape: a workspace-relative source path in a subdirectory, a
688+
// project-root RootFS, and an up-and-over link. The OS branch cannot
689+
// help (no on-disk file) and the legacy FS branch would hand ".." to
690+
// io/fs (which rejects it); RootFS-relative resolution collapses the
691+
// "..".
692+
func TestTargetExists_ParentTraversalViaRootFS(t *testing.T) {
693+
root := t.TempDir()
694+
require.NoError(t, os.MkdirAll(filepath.Join(root, "internal", "rules", "x"), 0o755))
695+
writeFile(t, filepath.Join(root, "internal", "rules", "x", "README.md"), "# X\n")
696+
697+
f := &lint.File{
698+
Path: "docs/background/linters.md",
699+
FS: os.DirFS(root),
700+
RootFS: os.DirFS(root),
701+
}
702+
assert.True(t, targetExists(f, "../../internal/rules/x/README.md", ""),
703+
"an existing up-and-over target must resolve via RootFS")
704+
assert.False(t, targetExists(f, "../../internal/rules/x/MISSING.md", ""),
705+
"a missing up-and-over target must still report absent")
706+
assert.False(t, targetExists(f, "../../../escape.md", ""),
707+
"a link escaping the workspace root must not resolve here")
708+
}
709+
710+
// TestResolveWorkspaceRelTarget covers every branch of the helper:
711+
// joining onto the source dir, collapsing "..", and the rejections for
712+
// empty, absolute, and root-escaping links.
713+
func TestResolveWorkspaceRelTarget(t *testing.T) {
714+
rel, ok := resolveWorkspaceRelTarget("docs/background/linters.md", "../../internal/x.md")
715+
require.True(t, ok)
716+
assert.Equal(t, "internal/x.md", rel)
717+
718+
rel, ok = resolveWorkspaceRelTarget("docs/a.md", "b.md")
719+
require.True(t, ok)
720+
assert.Equal(t, "docs/b.md", rel)
721+
722+
_, ok = resolveWorkspaceRelTarget("docs/a.md", "")
723+
assert.False(t, ok, "empty link is not a workspace target")
724+
_, ok = resolveWorkspaceRelTarget("docs/a.md", "/etc/passwd")
725+
assert.False(t, ok, "absolute link is not a workspace target")
726+
_, ok = resolveWorkspaceRelTarget("docs/a.md", "../../escape.md")
727+
assert.False(t, ok, "a link escaping the workspace root is rejected")
728+
}
729+
730+
// TestResolveTargetFile_ParentTraversalViaRootFS exercises the in-memory
731+
// RootFS branch of resolveTargetFile (the anchored-link path), which the
732+
// Session/WASM engine takes for an up-and-over target.
733+
func TestResolveTargetFile_ParentTraversalViaRootFS(t *testing.T) {
734+
root := t.TempDir()
735+
require.NoError(t, os.MkdirAll(filepath.Join(root, "internal", "rules", "x"), 0o755))
736+
writeFile(t, filepath.Join(root, "internal", "rules", "x", "README.md"), "# X\n")
737+
738+
f := &lint.File{
739+
Path: "docs/background/linters.md",
740+
FS: os.DirFS(root),
741+
RootFS: os.DirFS(root),
742+
}
743+
tf, ok := resolveTargetFile(f, "../../internal/rules/x/README.md", "")
744+
require.True(t, ok, "an existing up-and-over target must resolve via RootFS")
745+
data, err := tf.read()
746+
require.NoError(t, err)
747+
assert.Equal(t, []byte("# X\n"), data)
748+
749+
_, ok = resolveTargetFile(f, "../../internal/rules/x/MISSING.md", "")
750+
assert.False(t, ok, "a missing up-and-over target must not resolve")
751+
}
752+
686753
// =====================================================================
687754
// Additional coverage: toStringSlice with []string type
688755
// =====================================================================

0 commit comments

Comments
 (0)