fix(hopper): verify bookmark names and clear existing labels

Use the public rename API for an explicit empty existing name, retain omitted names and raw listing values, and verify fresh unnamed creation and explicit mutations by readback. Preserve independent verifier restorations and their primary failures.

Refs #5475. Focused bridge regressions and the complete real-Hopper suite passed on macOS arm64 with Hopper 6.6.0-demo 42.6316032. The verifier used a temporary unavailable Ghidra override to meet its sole-candidate prerequisite; reporter Hopper 6.1 remains unverified.

bridge/hopper_bridge.py

@@ -236,6 +236,15 @@ def _name_matches(document, address, name):
     return observed == name or (not name and observed is None)
 
 
+def _bookmark_name(document, address):
+    getter = getattr(document, "getBookmarkName", None)
+    if not callable(getter):
+        raise CapabilityUnavailableError(
+            "Hopper's public Python API does not expose getBookmarkName"
+        )
+    return getter(address)
+
+
 def _address(document, value=None):
     """Keep explicit hexadecimal coordinates distinct from symbol names."""
     if value is None:
@@ -994,15 +1003,46 @@ def _dispatch(method, params):
         observed = getter(address)
         return observed == comment or (comment == "" and observed is None)
     if method == "list_bookmarks":
-        return [{"address": _hex(item), "name": document.getBookmarkName(item)} for item in document.getBookmarks()]
+        return [{"address": _hex(item), "name": _bookmark_name(document, item)} for item in document.getBookmarks()]
     if method == "set_bookmark":
         address = _address(document, params.get("address"))
         _segment(document, address)
         name = params.get("name")
         if name is not None:
             _api_text(name, "Bookmark name")
-        document.setBookmarkAtAddress(address, name)
-        return document.hasBookmarkAtAddress(address)
+        has_bookmark = getattr(document, "hasBookmarkAtAddress", None)
+        if not callable(has_bookmark):
+            raise CapabilityUnavailableError(
+                "Hopper's public Python API does not expose hasBookmarkAtAddress"
+            )
+        exists = has_bookmark(address)
+        if name is None and exists:
+            # Omission means preserve the existing bookmark and its name.
+            return True
+        if not callable(getattr(document, "getBookmarkName", None)):
+            raise CapabilityUnavailableError(
+                "Hopper's public Python API does not expose getBookmarkName"
+            )
+        if name == "" and exists:
+            rename = getattr(document, "renameBookmarkAtAddress", None)
+            if not callable(rename):
+                raise CapabilityUnavailableError(
+                    "Hopper's public Python API does not expose renameBookmarkAtAddress"
+                )
+            rename(address, name)
+        else:
+            setter = getattr(document, "setBookmarkAtAddress", None)
+            if not callable(setter):
+                raise CapabilityUnavailableError(
+                    "Hopper's public Python API does not expose setBookmarkAtAddress"
+                )
+            setter(address, name)
+        if not has_bookmark(address):
+            return False
+        observed = _bookmark_name(document, address)
+        if name is None:
+            return observed is None or observed == ""
+        return observed == name or (name == "" and observed is None)
     if method == "unset_bookmark":
         address = _address(document, params.get("address"))
         # Permit removing a legacy orphan bookmark, but do not report success

scripts/lib/real-hopper-boundaries.mjs

@@ -96,7 +96,14 @@ export async function verifyHopperBoundaryContracts(
     ...(await verifyHopperSearch(call, invalid, procedure)),
     ...(await verifyHopperRegexIsolation(client, options, call, address)),
   };
+  const originalBookmark = (await call("list_bookmarks")).find(
+    (item) => item.address === address,
+  );
+  let bookmarkReplacement;
+  const boundaryFailures = [];
   try {
+    if (originalBookmark !== undefined)
+      assert.equal(await call("unset_bookmark", { address }), true);
     if (foreignDocument !== undefined) {
       await invalid(
         "read_bytes",
@@ -128,6 +135,49 @@ export async function verifyHopperBoundaryContracts(
       );
     }
 
+    assert.equal(
+      await call("set_bookmark", {
+        address,
+        name: "REA boundary bookmark before clear",
+      }),
+      true,
+    );
+    const beforeClear = (await call("list_bookmarks")).find(
+      (item) => item.address === address,
+    );
+    assert.deepEqual(beforeClear, {
+      address,
+      name: "REA boundary bookmark before clear",
+    });
+    assert.equal(await call("set_bookmark", { address, name: "" }), true);
+    const afterClear = (await call("list_bookmarks")).find(
+      (item) => item.address === address,
+    );
+    assert.deepEqual(afterClear, { address, name: null });
+
+    const replacementName = "REA boundary bookmark replacement";
+    assert.equal(
+      await call("set_bookmark", { address, name: replacementName }),
+      true,
+    );
+    const afterReplacement = (await call("list_bookmarks")).find(
+      (item) => item.address === address,
+    );
+    assert.deepEqual(afterReplacement, { address, name: replacementName });
+    assert.equal(await call("set_bookmark", { address }), true);
+    const afterOmittedName = (await call("list_bookmarks")).find(
+      (item) => item.address === address,
+    );
+    assert.deepEqual(afterOmittedName, {
+      address,
+      name: replacementName,
+    });
+    bookmarkReplacement = {
+      existingNameCleared: afterClear?.name === null,
+      nonemptyNameReplaced: afterReplacement?.name === replacementName,
+      omittedNamePreserved: afterOmittedName?.name === replacementName,
+    };
+
     for (const [setter, getter] of [
       ["set_comment", "comment"],
       ["set_inline_comment", "inline_comment"],
@@ -283,14 +333,51 @@ export async function verifyHopperBoundaryContracts(
       "idle",
       "a live GUI bridge must remain healthy after its launcher helper exits",
     );
+  } catch (error) {
+    boundaryFailures.push(error);
+    throw error;
   } finally {
-    await call("set_address_name", { address, name: originalName ?? "" });
-    await call("set_comment", { address, comment: originalComment ?? "" });
-    await call("set_inline_comment", {
-      address,
-      comment: originalInlineComment ?? "",
-    });
-    await call("unset_bookmark", { address });
+    const restorations = [
+      () => call("set_address_name", { address, name: originalName ?? "" }),
+      () => call("set_comment", { address, comment: originalComment ?? "" }),
+      () =>
+        call("set_inline_comment", {
+          address,
+          comment: originalInlineComment ?? "",
+        }),
+      async () => {
+        await call("unset_bookmark", { address });
+        if (originalBookmark === undefined) return;
+        assert.equal(
+          await call("set_bookmark", {
+            address,
+            ...(originalBookmark.name === null
+              ? {}
+              : { name: originalBookmark.name }),
+          }),
+          true,
+          "Could not restore the original disposable-target bookmark",
+        );
+        const restored = (await call("list_bookmarks")).find(
+          (item) => item.address === address,
+        );
+        assert.deepEqual(restored, originalBookmark);
+      },
+    ];
+    const restorationErrors = [];
+    for (const restore of restorations) {
+      try {
+        await restore();
+      } catch (error) {
+        restorationErrors.push(error);
+      }
+    }
+    if (restorationErrors.length > 0) {
+      throw new AggregateError(
+        [...boundaryFailures, ...restorationErrors],
+        "Could not restore all disposable Hopper boundary state",
+      );
+    }
   }
   return {
     schemaCount: catalog.tools.length,
@@ -301,6 +388,7 @@ export async function verifyHopperBoundaryContracts(
     navigationAndText,
     workflows,
     search,
+    bookmarkReplacement,
   };
 }
 

src/contracts/officialToolContracts.ts

@@ -220,7 +220,7 @@ export const OFFICIAL_TOOL_CONTRACTS = [
   ),
   official(
     "set_bookmark",
-    "Create or replace a bookmark at a mapped hexadecimal address and report success. This mutates navigation metadata; bookmarks are analyst-authored navigation aids, not binary evidence.",
+    "Create or update a bookmark at a mapped hexadecimal address and report readback success. Omit name to preserve an existing name or create an unnamed bookmark; an empty string clears the bookmark name. This mutates navigation metadata; bookmarks are analyst-authored navigation aids, not binary evidence.",
     z.object({ address, name: z.string().optional(), document }),
   ),
   official(

tests/boundary/providers/hopper/hopperBookmarks.test.ts

@@ -0,0 +1,114 @@
+import { execFile } from "node:child_process";
+import { fileURLToPath } from "node:url";
+import { promisify } from "node:util";
+
+import { describe, expect, it } from "vitest";
+import { z } from "zod";
+
+const execute = promisify(execFile);
+const bridgePath = fileURLToPath(
+  new URL("../../../../bridge/hopper_bridge.py", import.meta.url),
+);
+const probePath = fileURLToPath(
+  new URL("../../../fixtures/hopperBookmarkMutationProbe.py", import.meta.url),
+);
+
+const bookmark = z.strictObject({
+  address: z.literal("0x401000"),
+  name: z.string().nullable(),
+});
+const bookmarks = z.array(bookmark);
+const resultSchema = z.strictObject({
+  fresh_empty: z.strictObject({
+    result: z.boolean(),
+    listing: bookmarks,
+    raw_name: z.string().nullable(),
+  }),
+  default_named_omitted: z.strictObject({
+    result: z.boolean(),
+    listing: bookmarks,
+  }),
+  fresh_omitted: z.strictObject({ result: z.boolean(), listing: bookmarks }),
+  named_creation_result: z.boolean(),
+  empty_clear: z.strictObject({ result: z.boolean(), listing: bookmarks }),
+  nonempty_replacement: z.strictObject({
+    result: z.boolean(),
+    listing: bookmarks,
+  }),
+  omitted_existing: z.strictObject({
+    result: z.boolean(),
+    setter_calls_before: z.number().int(),
+    setter_calls_after: z.number().int(),
+    listing: bookmarks,
+  }),
+  ignored_setter: z.strictObject({ result: z.boolean(), listing: bookmarks }),
+  failed_clear: z.strictObject({ result: z.boolean(), listing: bookmarks }),
+  missing_rename: z.strictObject({
+    type: z.literal("CapabilityUnavailableError"),
+    diagnostic_type: z.literal("capability_unavailable"),
+  }),
+  preserved_after_missing_rename: bookmarks,
+  unset: z.strictObject({ result: z.boolean(), listing: bookmarks }),
+  rename_calls: z.array(z.tuple([z.string(), z.string()])),
+});
+
+describe("Hopper bookmark bridge mutations", () => {
+  it("uses exact provider readback and preserves omission semantics", async () => {
+    const { stdout } = await execute("python3", [probePath, bridgePath], {
+      encoding: "utf8",
+      timeout: 3_000,
+      maxBuffer: 1_024 * 1_024,
+    });
+    const result = resultSchema.parse(JSON.parse(stdout));
+
+    expect(result.fresh_empty).toEqual({
+      result: true,
+      listing: [{ address: "0x401000", name: "" }],
+      raw_name: "",
+    });
+    expect(result.fresh_omitted).toEqual({
+      result: true,
+      listing: [{ address: "0x401000", name: null }],
+    });
+    expect(result.default_named_omitted).toEqual({
+      result: false,
+      listing: [{ address: "0x401000", name: "provider-default" }],
+    });
+    expect(result.named_creation_result).toBe(true);
+    expect(result.empty_clear).toEqual({
+      result: true,
+      listing: [{ address: "0x401000", name: null }],
+    });
+    expect(result.nonempty_replacement).toEqual({
+      result: true,
+      listing: [{ address: "0x401000", name: "replacement-name" }],
+    });
+    expect(result.omitted_existing).toMatchObject({
+      result: true,
+      listing: [{ address: "0x401000", name: "replacement-name" }],
+    });
+    expect(result.omitted_existing.setter_calls_after).toBe(
+      result.omitted_existing.setter_calls_before,
+    );
+    expect(result.ignored_setter).toEqual({
+      result: false,
+      listing: [{ address: "0x401000", name: "replacement-name" }],
+    });
+    expect(result.failed_clear).toEqual({
+      result: false,
+      listing: [{ address: "0x401000", name: "replacement-name" }],
+    });
+    expect(result.missing_rename).toEqual({
+      type: "CapabilityUnavailableError",
+      diagnostic_type: "capability_unavailable",
+    });
+    expect(result.preserved_after_missing_rename).toEqual([
+      { address: "0x401000", name: "replacement-name" },
+    ]);
+    expect(result.unset).toEqual({ result: true, listing: [] });
+    expect(result.rename_calls).toEqual([
+      ["0x401000", ""],
+      ["0x401000", ""],
+    ]);
+  });
+});

tests/fixtures/hopperBookmarkMutationProbe.py

@@ -0,0 +1,184 @@
+"""Exercise bookmark mutation and exact readback at the Hopper bridge boundary."""
+
+import json
+from pathlib import Path
+import sys
+
+
+class FakeSegment:
+    pass
+
+
+class FakeDocument:
+    def __init__(self):
+        self.names = {}
+        self.setter_calls = []
+        self.rename_calls = []
+        self.ignore_next_set = False
+        self.ignore_next_rename = False
+        self.default_name_on_omitted = False
+
+    def getDocumentName(self):
+        return "bookmark-fixture"
+
+    def getExecutableFilePath(self):
+        return "/tmp/rea-hopper-bookmark-fixture"
+
+    def getDatabaseFilePath(self):
+        return None
+
+    def backgroundProcessActive(self):
+        return False
+
+    def getSegmentAtAddress(self, address):
+        return FakeSegment() if address == 0x401000 else None
+
+    def setBookmarkAtAddress(self, address, name=None):
+        self.setter_calls.append((address, name))
+        if self.ignore_next_set:
+            self.ignore_next_set = False
+            return True
+        if address not in self.names:
+            if name is None and self.default_name_on_omitted:
+                self.names[address] = "provider-default"
+            else:
+                self.names[address] = "" if name == "" else name
+        elif name is not None and name != "":
+            self.names[address] = name
+        # The native API return is not authoritative; the bridge reads state back.
+        return False
+
+    def renameBookmarkAtAddress(self, address, name):
+        self.rename_calls.append((address, name))
+        if self.ignore_next_rename:
+            self.ignore_next_rename = False
+            return False
+        self.names[address] = None if name == "" else name
+        return False
+
+    def hasBookmarkAtAddress(self, address):
+        return address in self.names
+
+    def getBookmarkName(self, address):
+        return self.names[address]
+
+    def getBookmarks(self):
+        return list(self.names)
+
+    def removeBookmarkAtAddress(self, address):
+        self.names.pop(address, None)
+        return False
+
+
+class FakeDocumentProvider:
+    document = FakeDocument()
+
+    @classmethod
+    def getAllDocuments(cls):
+        return [cls.document]
+
+    @classmethod
+    def getCurrentDocument(cls):
+        return cls.document
+
+
+def main():
+    bridge_path = sys.argv[1]
+    namespace = {"__file__": bridge_path, "__name__": "rea_hopper_bridge"}
+    source = Path(bridge_path).read_text(encoding="utf-8")
+    exec(compile(source, bridge_path, "exec"), namespace)
+    namespace["REA_TARGET_PATH"] = FakeDocumentProvider.document.getExecutableFilePath()
+    namespace["_configure_hopper_api"](FakeDocumentProvider)
+    namespace["_bind_session_document"]()
+    dispatch = namespace["_dispatch"]
+    address = "0x401000"
+    document = FakeDocumentProvider.document
+
+    fresh_empty_result = dispatch("set_bookmark", {"address": address, "name": ""})
+    fresh_empty_listing = dispatch("list_bookmarks", {})
+    fresh_empty_raw_name = document.getBookmarkName(0x401000)
+    dispatch("unset_bookmark", {"address": address})
+
+    document.default_name_on_omitted = True
+    default_named_omitted_result = dispatch("set_bookmark", {"address": address})
+    default_named_omitted_listing = dispatch("list_bookmarks", {})
+    dispatch("unset_bookmark", {"address": address})
+    document.default_name_on_omitted = False
+
+    fresh_omitted_result = dispatch("set_bookmark", {"address": address})
+    fresh_omitted_listing = dispatch("list_bookmarks", {})
+    dispatch("unset_bookmark", {"address": address})
+
+    named_creation_result = dispatch(
+        "set_bookmark", {"address": address, "name": "initial-name"}
+    )
+    clear_result = dispatch("set_bookmark", {"address": address, "name": ""})
+    clear_listing = dispatch("list_bookmarks", {})
+    renamed_result = dispatch(
+        "set_bookmark", {"address": address, "name": "replacement-name"}
+    )
+    replacement_listing = dispatch("list_bookmarks", {})
+    setter_calls_before_omitted = len(document.setter_calls)
+    omitted_result = dispatch("set_bookmark", {"address": address})
+    setter_calls_after_omitted = len(document.setter_calls)
+    omitted_listing = dispatch("list_bookmarks", {})
+
+    document.ignore_next_set = True
+    ignored_set_result = dispatch(
+        "set_bookmark", {"address": address, "name": "ignored-name"}
+    )
+    ignored_set_listing = dispatch("list_bookmarks", {})
+
+    document.ignore_next_rename = True
+    failed_clear_result = dispatch("set_bookmark", {"address": address, "name": ""})
+    failed_clear_listing = dispatch("list_bookmarks", {})
+
+    rename_method = document.renameBookmarkAtAddress
+    document.renameBookmarkAtAddress = None
+    missing_rename = None
+    try:
+        dispatch("set_bookmark", {"address": address, "name": ""})
+    except Exception as error:
+        missing_rename = {
+            "type": type(error).__name__,
+            "diagnostic_type": namespace["_diagnostic_type"](error),
+        }
+    document.renameBookmarkAtAddress = rename_method
+    preserved_after_missing_rename = dispatch("list_bookmarks", {})
+    unset_result = dispatch("unset_bookmark", {"address": address})
+    after_unset = dispatch("list_bookmarks", {})
+
+    print(json.dumps({
+        "fresh_empty": {
+            "result": fresh_empty_result,
+            "listing": fresh_empty_listing,
+            "raw_name": fresh_empty_raw_name,
+        },
+        "fresh_omitted": {
+            "result": fresh_omitted_result,
+            "listing": fresh_omitted_listing,
+        },
+        "default_named_omitted": {
+            "result": default_named_omitted_result,
+            "listing": default_named_omitted_listing,
+        },
+        "named_creation_result": named_creation_result,
+        "empty_clear": {"result": clear_result, "listing": clear_listing},
+        "nonempty_replacement": {"result": renamed_result, "listing": replacement_listing},
+        "omitted_existing": {
+            "result": omitted_result,
+            "setter_calls_before": setter_calls_before_omitted,
+            "setter_calls_after": setter_calls_after_omitted,
+            "listing": omitted_listing,
+        },
+        "ignored_setter": {"result": ignored_set_result, "listing": ignored_set_listing},
+        "failed_clear": {"result": failed_clear_result, "listing": failed_clear_listing},
+        "missing_rename": missing_rename,
+        "preserved_after_missing_rename": preserved_after_missing_rename,
+        "unset": {"result": unset_result, "listing": after_unset},
+        "rename_calls": [[hex(item[0]), item[1]] for item in document.rename_calls],
+    }, sort_keys=True))
+
+
+if __name__ == "__main__":
+    main()