Address Greptile feedback on bookmarks plugin

- Escape LIKE metacharacters in the search path so a query like "100%"
  no longer matches every bookmark; queries gain ESCAPE '\\' on the
  three OR'd LIKE clauses.
- Tighten MAX_TAGS check to fire BEFORE pushing the over-limit tag so
  the tags array is never momentarily oversized.
- Tombstone the markdown sidecar on delete (the SDK
  PluginLocalFoldersClient surface does not expose a remove primitive,
  so an atomic rewrite is the closest we can get without extending the
  SDK). Document the limitation inline.
- Extend the in-memory test harness to honor the LIKE escape character
  and add coverage for wildcard literal handling, MAX_TAGS upper bound,
  and the tombstone-on-delete path.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

packages/plugins/examples/plugin-bookmarks-example/src/worker.ts

@@ -105,15 +105,19 @@ function normalizeTags(value: unknown): string[] {
       throw new Error(`tag "${entry}" must use lowercase letters, digits, hyphens, or underscores`);
     }
     if (seen.has(cleaned)) continue;
-    seen.add(cleaned);
-    tags.push(cleaned);
-    if (tags.length > MAX_TAGS) {
+    if (tags.length >= MAX_TAGS) {
       throw new Error(`bookmarks accept at most ${MAX_TAGS} tags`);
     }
+    seen.add(cleaned);
+    tags.push(cleaned);
   }
   return tags;
 }
 
+function escapeLikePattern(value: string): string {
+  return value.replace(/[\\%_]/g, (char) => `\\${char}`);
+}
+
 export function deriveSlugCandidate(url: string, title: string | null): string {
   const titleSource = title?.trim();
   if (titleSource) {
@@ -217,9 +221,12 @@ async function listBookmarks(
   const params: unknown[] = [companyId];
   let where = "company_id = $1";
   if (search) {
-    params.push(`%${search.toLowerCase()}%`);
+    params.push(`%${escapeLikePattern(search.toLowerCase())}%`);
     const idx = params.length;
-    where += ` AND (lower(title) LIKE $${idx} OR lower(url) LIKE $${idx} OR lower(notes) LIKE $${idx})`;
+    where +=
+      ` AND (lower(title) LIKE $${idx} ESCAPE '\\'` +
+      ` OR lower(url) LIKE $${idx} ESCAPE '\\'` +
+      ` OR lower(notes) LIKE $${idx} ESCAPE '\\')`;
   }
   if (tag) {
     params.push(tag);
@@ -305,9 +312,42 @@ export async function deleteBookmark(
     `DELETE FROM ${tableName(ctx.db.namespace)} WHERE company_id = $1 AND slug = $2`,
     [companyId, slug],
   );
+  if (result.rowCount > 0) {
+    // The current PluginLocalFoldersClient surface (declarations / configure /
+    // status / list / readText / writeTextAtomic) does not expose a remove
+    // primitive, so we cannot atomically delete the markdown sidecar from the
+    // local folder when the row is removed. Overwrite the file with a
+    // tombstone marker instead so operators pointing the folder at a
+    // version-controlled directory can see the deletion without accumulating
+    // unbounded ghost files. The DB row remains the source of truth.
+    try {
+      await ctx.localFolders.writeTextAtomic(
+        companyId,
+        BOOKMARKS_FOLDER_KEY,
+        bookmarkFilePath(slug),
+        renderTombstoneMarkdown(slug),
+      );
+    } catch (error) {
+      ctx.logger.warn("Failed to tombstone bookmark sidecar after delete", {
+        slug,
+        error: error instanceof Error ? error.message : String(error),
+      });
+    }
+  }
   return { deleted: result.rowCount > 0, slug };
 }
 
+function renderTombstoneMarkdown(slug: string): string {
+  return [
+    "---",
+    `slug: ${slug}`,
+    "deleted: true",
+    `deleted_at: ${new Date().toISOString()}`,
+    "---",
+    "",
+  ].join("\n");
+}
+
 let listHandler: ((companyId: string, search: string | null, tag: string | null, limit: number) => Promise<BookmarkListResult>) | null = null;
 let createHandler: ((companyId: string, input: BookmarkInput) => Promise<BookmarkRecord>) | null = null;
 let deleteHandler: ((companyId: string, slug: string) => Promise<{ deleted: boolean; slug: string }>) | null = null;

packages/plugins/examples/plugin-bookmarks-example/tests/plugin.spec.ts

@@ -33,7 +33,8 @@ function makeInMemoryBookmarksDb() {
       let filtered = rows.filter((row) => row.company_id === companyId);
       if (sql.includes("LIKE $2")) {
         const like = (params?.[1] as string) ?? "";
-        const needle = like.replace(/%/g, "").toLowerCase();
+        const inner = like.startsWith("%") && like.endsWith("%") ? like.slice(1, -1) : like;
+        const needle = inner.replace(/\\(.)/g, "$1").toLowerCase();
         filtered = filtered.filter((row) =>
           row.title.toLowerCase().includes(needle) ||
           row.url.toLowerCase().includes(needle) ||
@@ -86,11 +87,31 @@ function makeInMemoryBookmarksDb() {
   };
 }
 
+interface RecordedFolderWrite {
+  companyId: string;
+  folderKey: string;
+  relativePath: string;
+  contents: string;
+}
+
 function makeHarness() {
   const harness = createTestHarness({ manifest });
   const db = makeInMemoryBookmarksDb();
   harness.ctx.db = db;
-  return { harness, db };
+  const folderWrites: RecordedFolderWrite[] = [];
+  const baseWriteTextAtomic = harness.ctx.localFolders.writeTextAtomic.bind(
+    harness.ctx.localFolders,
+  );
+  harness.ctx.localFolders.writeTextAtomic = async (
+    companyId,
+    folderKey,
+    relativePath,
+    contents,
+  ) => {
+    folderWrites.push({ companyId, folderKey, relativePath, contents });
+    return baseWriteTextAtomic(companyId, folderKey, relativePath, contents);
+  };
+  return { harness, db, folderWrites };
 }
 
 describe("plugin-bookmarks-example manifest", () => {
@@ -247,6 +268,99 @@ describe("plugin-bookmarks-example worker", () => {
     expect(after.bookmarks.map((b) => b.slug)).toEqual(["example-one"]);
   });
 
+  it("tombstones the markdown sidecar when a bookmark is deleted", async () => {
+    const companyId = "99999999-9999-9999-9999-999999999999";
+    const { harness, folderWrites } = makeHarness();
+    await plugin.definition.setup(harness.ctx);
+
+    await harness.performAction<BookmarkRecord>("create", {
+      companyId,
+      url: "https://example.com/del",
+      title: "Will be removed",
+    });
+    folderWrites.length = 0;
+
+    await harness.performAction<{ deleted: boolean; slug: string }>("delete", {
+      companyId,
+      slug: "will-be-removed",
+    });
+
+    expect(folderWrites).toHaveLength(1);
+    const tombstone = folderWrites[0]!;
+    expect(tombstone.relativePath).toBe("bookmarks/will-be-removed.md");
+    expect(tombstone.contents).toContain("deleted: true");
+    expect(tombstone.contents).toContain("slug: will-be-removed");
+  });
+
+  it("does not tombstone when no row matched the slug", async () => {
+    const companyId = "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa";
+    const { harness, folderWrites } = makeHarness();
+    await plugin.definition.setup(harness.ctx);
+    folderWrites.length = 0;
+
+    const result = await harness.performAction<{ deleted: boolean; slug: string }>(
+      "delete",
+      { companyId, slug: "missing" },
+    );
+    expect(result.deleted).toBe(false);
+    expect(folderWrites).toEqual([]);
+  });
+
+  it("treats LIKE wildcards in search input as literals", async () => {
+    const companyId = "77777777-7777-7777-7777-777777777777";
+    const { harness } = makeHarness();
+    await plugin.definition.setup(harness.ctx);
+
+    await harness.performAction<BookmarkRecord>("create", {
+      companyId,
+      url: "https://example.com/100-percent",
+      title: "100% complete",
+    });
+    await harness.performAction<BookmarkRecord>("create", {
+      companyId,
+      url: "https://example.com/other",
+      title: "Other entry",
+    });
+
+    const matchingPercent = await harness.getData<BookmarkListResult>("list", {
+      companyId,
+      search: "100%",
+    });
+    expect(matchingPercent.bookmarks.map((b) => b.slug)).toEqual(["100-complete"]);
+
+    const matchingUnderscore = await harness.getData<BookmarkListResult>("list", {
+      companyId,
+      search: "_",
+    });
+    expect(matchingUnderscore.bookmarks).toEqual([]);
+  });
+
+  it("caps tag count exactly at MAX_TAGS without holding an oversized array", async () => {
+    const companyId = "88888888-8888-8888-8888-888888888888";
+    const { harness, db } = makeHarness();
+    await plugin.definition.setup(harness.ctx);
+
+    const sixteenTags = Array.from({ length: 16 }, (_, i) => `t${i}`);
+    const created = await harness.performAction<BookmarkRecord>("create", {
+      companyId,
+      url: "https://example.com/tags",
+      title: "tagged",
+      tags: sixteenTags,
+    });
+    expect(created.tags).toHaveLength(16);
+
+    await expect(
+      harness.performAction("create", {
+        companyId,
+        url: "https://example.com/too-many",
+        title: "too-many",
+        tags: [...sixteenTags, "extra"],
+      }),
+    ).rejects.toThrow(/at most 16 tags/);
+
+    expect(db.rows).toHaveLength(1);
+  });
+
   it("rejects URLs without an http(s) scheme", async () => {
     const { harness } = makeHarness();
     await plugin.definition.setup(harness.ctx);