Skip to content
Merged
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
8 changes: 4 additions & 4 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,10 @@ The **one typed API**; the CLI and the desktop host are its only clients (every
module is called directly only by integration tests). Surface: lifecycle + indexing (`open` /
`open_with_embedder` / `reindex` / `reindex_with_progress` / `plan_reindex` / `project` / `embed`),
reads (`read` / `list_notes` / `list_resources` / `list_dirs` / `neighbors` / `explain` /
`explain_resource` / `search` / `search_evidence` / `similar` / `ask`), writes (`add_note` /
`create_note` / `create_dir` / `import_file` / `import_path` / `move_note` / `move_resource` /
`move_dir` / `link` / `write` / `write_frontmatter` / `delete_note` / `delete_resource` /
`delete_dir`). **Add operations when a command needs them; do not pre-build a broad surface.** The
`explain_resource` / `read_resource_bytes` / `search` / `search_evidence` / `similar` /
`ask`), writes (`add_note` / `create_note` / `create_dir` / `import_file` / `import_path` /
`move_note` / `move_resource` / `move_dir` / `link` / `write` / `write_frontmatter` /
`delete_note` / `delete_resource` / `delete_dir`). **Add operations when a command needs them; do not pre-build a broad surface.** The
embedder is injected here: `open` defaults to the fake, `open_with_embedder` wires the real model.

### Data flows
Expand Down
20 changes: 20 additions & 0 deletions crates/b2-core/src/vault.rs
Original file line number Diff line number Diff line change
Expand Up @@ -895,6 +895,26 @@ impl Vault {
dirs::list_dirs(&self.root)
}

/// Read a resource's **bytes** — [`read`](Self::read)'s non-note sibling, for the
/// viewers an adapter shows in place of the fallback card (an image, today).
///
/// Inventory-checked *before* the filesystem is touched, the same posture the
/// desktop's *Open in system default* takes: the path must name a row the walk put
/// in `resources`, so this can never be talked into "read any file this process can
/// reach" by a link a note authored. Errors with [`Error::ResourceNotFound`]
/// otherwise — including for a note, which is not resource inventory.
///
/// Reads whole, because every caller wants the whole file; a resource too large for
/// a viewer is the *adapter's* judgement (it knows the size from
/// [`explain_resource`](Self::explain_resource) before asking), not a rule the
/// engine imposes.
pub fn read_resource_bytes(&self, path: &str) -> Result<Vec<u8>> {
let _op = tracing::debug_span!(target: "b2::vault", "read_resource_bytes", path).entered();
db::resource_detail(&self.conn, path)?
.ok_or_else(|| Error::ResourceNotFound(path.to_string()))?;
Ok(fs::read(self.root.join(path))?)
}

/// The fallback card's data for one resource: inventory metadata plus the
/// backlinks panel, straight off the materialized graph. `path` is vault-relative
/// (the adapters dispatch here via [`crate::resource::doc_kind`]); errors with
Expand Down
34 changes: 34 additions & 0 deletions crates/b2-core/tests/resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,40 @@ fn explain_resource_carries_metadata_and_backlinks() {
assert!(matches!(missing, Err(b2_core::Error::ResourceNotFound(_))));
}

/// The viewer's read: inventoried bytes come back verbatim; anything the walk never
/// put in the inventory refuses, so the op cannot be steered off the vault.
#[test]
fn read_resource_bytes_returns_the_file_and_refuses_the_uninventoried() {
let tmp = tempfile::TempDir::new().unwrap();
common::golden_vault_copy(tmp.path());
let vault = Vault::open(tmp.path()).unwrap();
vault.project(false).unwrap();

let bytes = vault.read_resource_bytes("resources/diagram.png").unwrap();
assert_eq!(
bytes,
fs::read(tmp.path().join("resources/diagram.png")).unwrap(),
"the viewer sees exactly the file on disk"
);

// A path the walk never met: no row, no read — whatever is on disk beside it.
fs::write(tmp.path().join("resources/unwalked.png"), b"not indexed").unwrap();
assert!(matches!(
vault.read_resource_bytes("resources/unwalked.png"),
Err(b2_core::Error::ResourceNotFound(_))
));

// A note is not resource inventory, and an escaping path names no row at all.
assert!(matches!(
vault.read_resource_bytes("notes/alpha.md"),
Err(b2_core::Error::ResourceNotFound(_))
));
assert!(matches!(
vault.read_resource_bytes("../outside.png"),
Err(b2_core::Error::ResourceNotFound(_))
));
}

/// A resource move rewrites inbound links in BOTH syntaxes, each keeping its own
/// convention (note-relative stays relative, vault-root stays root), moves the
/// file, and leaves the index equal to a fresh rebuild.
Expand Down
18 changes: 18 additions & 0 deletions crates/b2-desktop/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,24 @@ pub fn explain_resource(
Ok(vault.explain_resource(&path)?)
}

/// A resource's **bytes**, base64, for the viewer the card shows in place of the
/// *No viewer available* fallback (an image, today).
///
/// base64 because the IPC is JSON and these are arbitrary bytes — the same encoding
/// [`import_file`] takes in the other direction, and what the webview turns straight
/// into the `data:` URL the CSP already admits (`img-src 'self' data:`). It carries a
/// copy of the file, so the frontend asks only for what it will actually display: the
/// size is on the card's view before this is ever called.
///
/// The path is re-validated host-side against the inventory (in the façade), because
/// the note that authored the link is untrusted input (ADR-0016) — the caller passes,
/// the host validates, exactly as [`open_resource`] does.
#[tauri::command(async)]
pub fn read_resource(state: State<'_, AppState>, path: String) -> Result<String, CmdError> {
let vault = open_read(state.inner())?;
Ok(BASE64.encode(vault.read_resource_bytes(&path)?))
}

/// *Open in system default* on the fallback card — an **OS handoff**, never
/// in-webview execution (spec §6 security posture). Host infrastructure like the
/// folder dialog: the webview holds no opener permission; this command validates
Expand Down
1 change: 1 addition & 0 deletions crates/b2-desktop/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,7 @@ fn main() {
commands::list_dirs,
commands::list_resources,
commands::explain_resource,
commands::read_resource,
commands::open_resource,
commands::open_external,
commands::clipboard_text,
Expand Down
9 changes: 9 additions & 0 deletions ui/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,15 @@ export const api = {
explainResource: (path: string): Promise<ResourceExplainView> =>
invoke("explain_resource", { path }),

/**
* A resource's bytes, base64 — what the in-app viewer shows in place of the
* *No viewer available* card. This carries a copy of the whole file across the IPC,
* so ask only for what will actually be displayed; the size is on
* `explainResource`'s view before this is called. The host re-validates the path
* against the inventory.
*/
readResource: (path: string): Promise<string> => invoke("read_resource", { path }),

/**
* *Open in system default* — an OS handoff performed host-side (the webview holds
* no opener permission); the host validates the path against the inventory first.
Expand Down
87 changes: 87 additions & 0 deletions ui/src/embedlink.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
// Tests for the `[[wikilink]]` / `![[embed]]` grammar in the reading view (render.ts's
// `marked` extension). Run directly:
// node --experimental-strip-types src/embedlink.test.ts
// Hand-rolled asserts, the sanitize.test.ts idiom.
//
// Through `renderMarkdown` rather than the extension object, because the claim under test
// is what a note *looks like on screen*, and that is the composition of the extension, the
// tokenizer ordering against marked's own image rule, and the sanitizer hook. The
// extension in isolation would still pass with the `start` hook mis-anchored, which is
// exactly the bug that put a stray `!` in front of every embed.
//
// jsdom supplies the DOM DOMPurify parses with — a devDependency, nothing ships it.

import { JSDOM } from "jsdom";
import { renderMarkdown } from "./render.ts";

(globalThis as unknown as { window: unknown }).window = new JSDOM("").window;

let checks = 0;

function assertHas(haystack: string, needle: string, label: string): void {
if (!haystack.includes(needle)) {
throw new Error(`${label}\n missing: ${needle}\n in: ${haystack}`);
}
checks++;
}

function assertNot(haystack: string, needle: string, label: string): void {
if (haystack.includes(needle)) {
throw new Error(`${label}\n found: ${needle}\n in: ${haystack}`);
}
checks++;
}

// --- the plain wikilink: target carried, label shown ----------------------------------

const plain = renderMarkdown("see [[concepts/memory]] today\n");
assertHas(plain, 'data-target="concepts/memory"', "a bare wikilink carries its target");
assertHas(plain, ">concepts/memory</a>", "…and labels itself with it");

const labelled = renderMarkdown("see [[concepts/memory|how it works]]\n");
assertHas(labelled, 'data-target="concepts/memory"', "a `|` alias leaves the target alone");
assertHas(labelled, ">how it works</a>", "…and is what the reader sees");

// --- the embed form: the `!` is markup, not text --------------------------------------
//
// `![[file]]` is the same link with the core's embed marker in front (link.rs). B2 has no
// inline viewer for it yet, so it renders as the link — but the `!` is *grammar*, and a
// grammar character that leaks into the prose is a rendering bug, not a partial feature.

const embed = renderMarkdown("![[__Attachments/Screenshot 1.png]]\n");
assertNot(embed, ">!", "the embed marker never reaches the reader as text");
assertNot(embed, "<p>!", "…not even at the head of its own paragraph");
assertHas(embed, 'data-target="__Attachments/Screenshot 1.png"', "the embed carries its target");
assertHas(embed, ">__Attachments/Screenshot 1.png</a>", "…and labels itself with it");

// --- an embed's `|`-part is a size hint, not a label ----------------------------------
//
// `![[img.png|400]]` asks for a 400px-wide render. B2 does not size images, and the
// unsupported half must not eat the supported one: dropping the hint is right, replacing
// the filename with "400" is the bug this pins.

const sized = renderMarkdown("![[__Attachments/Screenshot 2.png|400]]\n");
assertHas(sized, 'data-target="__Attachments/Screenshot 2.png"', "a sized embed keeps its target");
assertHas(sized, ">__Attachments/Screenshot 2.png</a>", "…and still reads as the filename");
assertNot(sized, ">400<", "the size hint is dropped, not shown");
assertNot(sized, "<p>!", "…and the marker is still markup");

// --- neighbours the grammar must not swallow ------------------------------------------

assertHas(
renderMarkdown("![alt text](resources/diagram.png)\n"),
'<img src="resources/diagram.png"',
"Markdown's own image form is untouched — the `!` there belongs to marked",
);
assertHas(
renderMarkdown("a shout! [[concepts/memory]]\n"),
"a shout! ",
"a `!` that isn't a marker stays in the prose",
);
assertHas(
renderMarkdown("brackets [[not closed here\n"),
"[[not closed",
"an unclosed `[[` is prose, and the marker rule doesn't change that",
);

console.log(`embedlink.test.ts: ${checks} checks passed`);
38 changes: 36 additions & 2 deletions ui/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,13 @@ import { icon } from "./icons";
import { activeAfter, countLabel, FIND_CAP, findMatches, locate, stepActive, type Match } from "./findbar";
import { BOUNDS, initPanes } from "./panes";
import { reconcileIndex } from "./reconcile";
import type { ResourceExplainView } from "./types";
import {
contextMenuHtml,
embedBannerHtml,
escapeHtml,
imageDataUrl,
IMAGE_VIEWER_MAX_BYTES,
modalHtml,
notePaneHtml,
reindexDisabled,
Expand Down Expand Up @@ -541,6 +544,7 @@ async function loadNote(ref: string, commit: (path: string) => void): Promise<bo
const note = await api.readNote(ref);
state.current = note;
state.currentResource = null; // one document owns the pane
state.resourceImage = null;
state.fmEditing = false; // a new document ends any drawer edit (guards ran upstream)
commit(note.path);
expandAncestors(note.path);
Expand Down Expand Up @@ -598,6 +602,25 @@ async function followWikilink(target: string): Promise<void> {
}
}

/**
* The open resource's picture, or null when there isn't one to show.
*
* Null for every class without an in-app viewer, for an image too large to hold on
* screen (`IMAGE_VIEWER_MAX_BYTES` — the card's size is already in hand, so the decision
* costs no IPC), and for a read that failed. That last one is deliberate: the card is the
* truth about the file whether or not its bytes can be read, so a failed read falls back
* to *Open in system default* rather than failing the navigation and leaving the pane on
* the previous document.
*/
async function loadResourceImage(r: ResourceExplainView): Promise<string | null> {
if (r.class !== "image" || r.size > IMAGE_VIEWER_MAX_BYTES) return null;
try {
return imageDataUrl(r.path, await api.readResource(r.path));
} catch {
return null;
}
}

/** The resource sibling of `loadNote` — same core/commit split, for `openResource`
* and back/forward. Discovery doesn't apply (resources have no chunks until file-type
* slice 3), so the side pane clears. */
Expand All @@ -607,6 +630,7 @@ async function loadResource(path: string, commit: (path: string) => void): Promi
try {
const resource = await api.explainResource(path);
state.currentResource = resource;
state.resourceImage = await loadResourceImage(resource);
state.current = null;
commit(resource.path);
expandAncestors(resource.path);
Expand Down Expand Up @@ -1593,7 +1617,9 @@ async function executeMove(node: TreeNodeRef, to: string): Promise<boolean> {
state.current = await api.readNote(openNotePath);
}
if (openResourcePath !== null) {
state.currentResource = await api.explainResource(openResourcePath);
const moved = await api.explainResource(openResourcePath);
state.currentResource = moved;
state.resourceImage = await loadResourceImage(moved);
}
await loadNotes();
if (openNotePath !== null) await refreshDiscovery(); // backlinks may show new paths
Expand Down Expand Up @@ -1701,6 +1727,7 @@ async function executeDelete(node: TreeNodeRef): Promise<void> {
if (affected) {
state.current = null;
state.currentResource = null;
state.resourceImage = null;
state.similar = [];
state.connections = [];
state.resourceLinks = [];
Expand Down Expand Up @@ -2641,6 +2668,7 @@ async function switchVault(): Promise<void> {
state.notesTotal = info.notes_total;
state.current = null;
state.currentResource = null;
state.resourceImage = null;
state.similar = [];
state.connections = [];
state.resourceLinks = [];
Expand Down Expand Up @@ -3918,7 +3946,13 @@ async function reconcileExternalChange(): Promise<void> {
const cur = state.currentResource;
try {
const fresh = await api.explainResource(cur.path);
if (state.currentResource?.path === cur.path) state.currentResource = fresh;
// The bytes are re-read too: an external edit can rewrite the picture in place
// without the path ever changing, and a stale `data:` URL would show the old one.
const picture = await loadResourceImage(fresh);
if (state.currentResource?.path === cur.path) {
state.currentResource = fresh;
state.resourceImage = picture;
}
} catch {
if (state.currentResource?.path === cur.path) {
flash("This file is no longer on disk — it was moved or removed.");
Expand Down
Loading