Merge the IDA Code Mode port

Replaces the private idalib worker (idatui/worker.py + worker_client.py, with
server/patch_server.py injecting tools into ida-pro-mcp) with an ordinary
client of ida_codemode.client.DatabaseHandle. A database open by an IDA GUI is
reused; otherwise Code Mode starts or shares a managed idalib worker. The TUI
no longer owns an IDA process, and closing it releases only its lease.

Based on Duncan Ogilvie's port, rebased onto ~150 commits of local work it
predated. The rebase itself was mechanical; landing it was not. Nine defects
had to be fixed before the feature set was whole again, none of which the
patch's own tests could catch:

  - DatabaseHandle.open() takes image_base, not loading_address: every
    connect() would have raised TypeError on the first call
  - five operations our tree had grown were simply missing (flowchart, so the
    graph view was dead; op_format/pc_nums/pc_num_format, so 'o'/'O' were;
    survey_binary)
  - set_comments wrote only the disassembly comment, so comments never
    appeared in pseudocode
  - xref_query returned rows in raw IDA order, and 'follow the call' silently
    followed the fall-through instead
  - rename accepted one edit per category, so bulk symbol import was dead
  - decompile ran decomp_map's full per-column ctree sweep to fill in a
    per-line address anchor
  - heads shipped without operand extents or the digest protocol
  - the package became unimportable without ida_codemode installed, which
    killed the offline test suites

Verified against the pre-codemode tag rather than against assumptions: the
full suite is 788 passed / 0 failed, and the pilot's 301 checks match the old
backend exactly. Performance is within 2x on the listing hot path and faster
on decompile, disasm and connect, after fixing two runtime costs that are
documented for upstream in docs/CODEMODE_UPSTREAM.md.

Test runtime came down from ~9m20s to 115s along the way -- not by removing
checks, but by removing four kinds of waiting-on-a-guess that were also
hiding real failures.

1 files changed, 269 insertions, 0 deletions

diff --git a/docs/CODEMODE_UPSTREAM.md b/docs/CODEMODE_UPSTREAM.md
new file mode 100644
index 0000000..f903599
--- /dev/null
+++ b/docs/CODEMODE_UPSTREAM.md
@@ -0,0 +1,269 @@
+# Findings from porting a real client to IDA Code Mode
+
+Notes for the `ida-codemode` maintainers, gathered while porting **ida-tui** (a
+Textual TUI frontend for IDA) from a private idalib worker to
+`ida_codemode.client.DatabaseHandle`.
+
+Everything below is measured, not inferred. Where we worked around something, the
+workaround is named so you can judge whether the library should make it
+unnecessary.
+
+**Environment:** ida-codemode 0.3.1, IDA 9.4 (idalib), Linux, single managed
+worker backend, quiet box. Target for timings: `targets/echo` unless stated.
+
+**What the client does**, for scale: it renders a continuous disassembly listing,
+pseudocode, a CFG graph view and a hex view, paging over the database as the user
+scrolls. It is latency-sensitive in a way an agent-driven MCP client is not — a
+keypress must repaint. It issues ~1–8 operations per user action.
+
+---
+
+## 1. `timeout_trace` enables line tracing in every frame — 52x on IDA calls
+
+**Highest-impact item by a wide margin.**
+
+`runtime.py` wraps every `execute_python` in `sys.settrace(timeout_trace)` to
+enforce the deadline. `timeout_trace` ends with `return timeout_trace`, and
+returning a trace function from a `'call'` event asks CPython to trace **every
+line of that frame**. So every line of every function the snippet touches pays a
+Python-level callback, and the specialising interpreter is disabled throughout.
+
+Measured inside the worker, same process, same database:
+
+| | traced (stock) | untraced | native idalib |
+|---|---|---|---|
+| `ida_bytes.get_flags(ea)` | 5.49 µs | 0.106 µs | 0.119 µs |
+| our 200-row listing page | 20.2 ms | 2.0 ms | — |
+
+Untraced matches a plain idalib process, so the trace hook accounts for
+essentially all of it. For us this was the single largest cost in the port —
+larger than HTTP, serialisation and IDA itself combined.
+
+Reproduce inside any `execute_python`:
+
+```python
+import sys, time, ida_bytes
+def bench():
+    t = time.perf_counter()
+    for _ in range(20000): ida_bytes.get_flags(0x1000)
+    return (time.perf_counter() - t) / 20000 * 1e6
+traced = bench()
+old = sys.gettrace(); sys.settrace(None)
+try:    untraced = bench()
+finally: sys.settrace(old)
+result = {"traced_us": traced, "untraced_us": untraced}
+```
+
+**Suggested fixes, cheapest first**
+
+1. `return None` from `timeout_trace` instead of itself. You keep `'call'`-event
+   deadline checks — which is enough to interrupt anything that calls a function
+   — and drop per-line tracing entirely.
+2. On 3.12+, use `sys.monitoring` with only the events you need; it is designed
+   for exactly this and is far cheaper than `settrace`.
+3. Or drop the trace and rely on the `threading.Timer` →
+   `ida_kernwin.set_cancelled()` path you already have, accepting that a
+   pure-Python loop with no calls in it cannot be interrupted.
+
+**Our workaround** (we would rather not ship it): the snippet detaches the trace
+and restores it in a `finally`. That gives up deadline enforcement for
+pure-Python loops inside our own code; your native cancel timer is unaffected and
+still fires. Every client that does real work per call will eventually find this
+and do the same, which is an argument for fixing it in the runtime.
+
+---
+
+## 2. `to_jsonable` dominates any large result
+
+`execute_python` runs `to_jsonable()` over whatever the snippet returns. Our
+answers are already JSON-safe and they are big — a 200-row listing page is
+roughly 10k small objects.
+
+| | cost |
+|---|---|
+| `to_jsonable(page)` | 66.2 ms |
+| `json.dumps(page, separators=(",",":"))` — same data | 0.58 ms |
+| serialised size | 34.9 KB |
+
+That is 114x, and it was 72% of the page's total cost before we changed it.
+
+**Suggested fixes**
+
+- Fast-path values that are already JSON-safe (a cheap recursive type check that
+  bails to the original object beats rebuilding it), or
+- let a snippet opt out by returning an already-serialised payload — a documented
+  envelope such as `{"__json__": "<...>"}`, or simply passing `str`/`bytes`
+  through untouched.
+
+**Our workaround:** snippets `json.dumps` inside the database process and return
+one string, which the client parses. `to_jsonable` then walks a single scalar.
+Cost went 66.2 ms → ~0.6 ms. It works, but every client with a large result set
+has to discover and re-implement it.
+
+---
+
+## 3. The per-operation floor is `execute_sync`, not HTTP
+
+Same worker, same connection, 200 iterations:
+
+| | cost |
+|---|---|
+| `GET /health` (no `execute_sync`) | **0.165 ms** |
+| `execute_python("result = 1")` | **2.025 ms** |
+
+HTTP framing is ~7% of the floor; marshalling the operation onto IDA's main
+thread is the other ~93%. The worker runs IDA's own `kernwin.serve()`, so this is
+plausibly IDA's dispatch latency rather than anything you control — but it is
+worth **documenting**, because it sets a hard 2 ms per-operation budget that
+shapes how a client must be designed.
+
+It did not hurt us (our call volume is 1–8 per user action; 4 calls to build a
+1060-block graph), but a client that makes one call per row or per symbol will be
+20–100x slower than an in-process one and the authors will not know why.
+
+**Suggested fixes:** document the floor; and consider a batch endpoint — accept
+`[{op, args}, ...]` and dispatch them within a single `execute_sync` — which
+would let chatty clients amortise it without redesigning around it.
+
+---
+
+## 4. Loader switches on an existing database are a FATAL, not an error
+
+Opening a target that already has an `.i64`, while passing spawn-only options,
+kills the worker:
+
+```
+FATAL ERROR: @0:636[]
+Switch '-b400' can be used only when loading a new file
+```
+
+The client sees only:
+
+```
+IDAConnectionError: idalib worker launcher <pid> exited with status 1
+```
+
+This is easy to hit and hard to diagnose: it is the natural second run of
+anything that opens a raw blob (`processor=`/`image_base=`/`file_type=` are
+recorded in the database the first run produced). Our test suite hit it as a
+crash five minutes into a run.
+
+**Suggested fixes**
+
+- In `DatabaseHandle.open()`, when the resolved IDB already exists and
+  `new_database` is not set, either ignore the spawn-only options or raise a
+  typed error naming them — before handing them to IDA.
+- Propagate the worker's fatal text into the client exception. The message
+  already exists on the worker's stderr; losing it turns a one-line fix into a
+  bisect.
+
+**Our workaround:** the client checks whether the expected IDB exists and drops
+`processor`/`image_base`/`file_type` when it does.
+
+---
+
+## 5. Deleting or replacing an IDB under a live lease fails silently
+
+A suite that did "delete the `.i64`, reopen the same path" (safe when it owned a
+private worker) now races the previous worker's lease grace. The reopen produced
+a handle that never became usable, with no error — just a database with no
+listing, and every wait timing out.
+
+**Suggested fixes**
+
+- Detect that the IDB backing a registered instance has been removed or replaced
+  and fail loudly (the registry already holds `idb_key`).
+- Expose a **public** "wait until this database is released" primitive. We needed
+  one and ended up reaching into `registry.REGISTRY_DIR` and `FileLock` to build
+  it, which is not an API we should be depending on.
+- Document the lease-grace window as part of the lifecycle contract.
+
+---
+
+## 6. No close-without-save, and no rollback
+
+A managed worker saves when its final lease closes. A GUI handle leaves GUI state
+as-is. Neither gives a client a way to say "discard what I did".
+
+ida-tui had a "discard & quit" that we could not port; it is now "leave as-is &
+quit", and we cannot honestly promise the user their edits are not persisted.
+
+**Suggested fixes:** a close policy on a lease the client created
+(`close(save=False)`), or a transaction/rollback API, or a documented
+disposable-copy pattern that clients can follow.
+
+---
+
+## 7. No change notification for shared databases
+
+The lease reports liveness, not mutations. If a GUI user or another Code Mode
+client renames or retypes while we are attached, our materialised caches (name
+generation, decompilation, listing pages) are silently stale. Our own edits
+invalidate correctly; someone else's cannot.
+
+**Suggested fix — cheap and sufficient:** a monotonic database revision counter,
+bumped on any mutating operation and exposed on `/health` (and ideally on the
+lease event stream). Clients can then invalidate by comparing one integer. A full
+change feed would be better but is much more work; the counter alone would make
+shared editing safe for every caching client.
+
+---
+
+## 8. Package exports and API surface stability
+
+`ida_codemode/__init__.py` exports nothing, so a library consumer must import
+from submodules:
+
+```python
+from ida_codemode.client   import DatabaseHandle, ClientError, RemoteError, InstanceDisconnectedError
+from ida_codemode.registry import REGISTRY_DIR, FileLock, RegistryEntry, canonical_path, idb_key, scan_instances
+from ida_codemode.resolver import IdbBusy, expected_idb_path
+```
+
+Some of those are clearly internals (`FileLock`, `REGISTRY_DIR`) that we only
+touch because no public equivalent exists (see §5).
+
+**Suggested fix:** export `DatabaseHandle` and the public exception types from the
+package root, and mark the intended-public registry helpers explicitly. It also
+makes "what is API and what is internal" answerable, which right now it is not.
+
+---
+
+## 9. A testing note: `DatabaseHandle.open()`'s 30 keyword-only options
+
+The port we started from called `open(..., loading_address=...)`. The real
+parameter is `image_base`. Every `connect()` would have raised `TypeError` on the
+first call, and its contract tests passed anyway, because a hand-written fake
+handle accepts `**kwargs`.
+
+Not a library bug — but with 30 keyword-only options it is a very easy mistake,
+and it is invisible to exactly the offline tests people write.
+
+**Suggested fix:** ship `py.typed` and/or a `Protocol` for the handle, so a fake
+can be checked against the real signature and a typo is caught statically. (We
+added a test asserting our kwargs are a subset of
+`inspect.signature(DatabaseHandle.open).parameters`, which is a poor substitute.)
+
+---
+
+## Priority, from a client author's view
+
+| # | item | impact | fixable by you? |
+|---|---|---|---|
+| 1 | `timeout_trace` line tracing | 52x on IDA calls, 10x on real operations | yes, one line |
+| 2 | `to_jsonable` on large results | 114x on serialisation | yes |
+| 7 | no change/revision counter | correctness for shared editing | yes, cheap |
+| 4 | loader switches fatal on reopen | crashes, hard to diagnose | yes |
+| 5 | replaced/deleted IDB under lease | silent hang | yes |
+| 6 | no close-without-save | a feature we had to drop | design question |
+| 8 | package exports | forces internal imports | yes, trivial |
+| 3 | 2 ms `execute_sync` floor | shapes client design | document; maybe batch |
+| 9 | typed handle for fakes | catches a whole bug class | yes |
+
+Items 1 and 2 together were the difference between "the port is 35x slower than
+the private worker it replaced" and "the port is within 2x, and faster on several
+operations". Both are in the runtime, not in client code — which is why they are
+worth fixing centrally rather than leaving each client to rediscover.
+
+Happy to supply the benchmark harness (it is backend-agnostic and runs against
+both our old worker and Code Mode), or to test a patch.