ABI stability: exported FFI symbol signatures must not change — add versioned exports instead

[EnRaiha]

Summary

Exported FFI symbol signatures freeze at the first tagged release. Before that tag, change the signature in place and regenerate include/nodedb_lite.h. After that tag, add a separately named export; the legacy entry point delegates and discards the new output.

Context

PR #15 initially added an out_edge_id parameter to the existing nodedb_graph_insert_edge export. For a shipped binary this is an ABI break: a caller compiled against the old declaration still resolves the same symbol, but the callee reads a sixth register/stack argument the old caller never initialized — if it happens to be non-null, write_c_string can dereference an arbitrary address.

Nothing has shipped yet (no releases, no tags; the 0.1.0 heading in CHANGELOG.md is unreleased), so no binary anywhere was compiled against a five-argument nodedb_graph_insert_edge. Per the pre-release rule, #15 was merged with a single six-argument nodedb_graph_insert_edge (f9c520c) — no legacy alias for zero existing callers.

Follow-ups

• CONTRIBUTING.md paragraph stating the freeze point explicitly (first tagged release).
• CI check: diff nm -D --defined-only libnodedb_lite_ffi.so | sort against the previous release tag. No-op until the first tag exists, which is the correct behaviour.
• Bump nodedb_abi_version on any signature change so bindings fail fast.

[farhan-syah]

The rule is right. The trigger is wrong.

Nothing has shipped yet: gh release list is empty and the remote carries no tags. The 0.1.0 heading in CHANGELOG.md is unreleased. So no binary anywhere was compiled against a five-argument nodedb_graph_insert_edge, and the wild-write scenario has no caller to happen to.

That is why f9c520c on #15 collapses nodedb_graph_insert_edge_with_id back into a single six-argument nodedb_graph_insert_edge. A legacy alias kept for zero existing callers is permanent surface: two exports, two doc blocks, two test paths, and a name that says _with_id long after everyone uses it.

Proposed wording for the rule:

• Exported #[no_mangle] signatures freeze at the first tagged release.
• Before that tag, change the signature in place and regenerate include/nodedb_lite.h.
• After that tag, add a separately named export; the legacy entry point delegates and discards the new output.

The follow-ups stand on their own and are worth doing:

• The CONTRIBUTING.md paragraph, with the freeze point stated explicitly.
• The nm -D --defined-only libnodedb_lite_ffi.so | sort diff in CI, comparing against the previous release tag. It is a no-op until the first tag exists, which is the correct behaviour.
• Bumping nodedb_abi_version on any signature change, so bindings fail fast.

[EnRaiha]

Agreed — the trigger was wrong. Nothing has shipped (no releases, no tags; 0.1.0 unreleased), so there is no five-argument caller to protect. Rule corrected: signatures freeze at the first tagged release; before that tag, change in place and regenerate the header. Issue body updated to the corrected wording. Follow-ups (CONTRIBUTING paragraph, nm -D CI diff vs previous tag, nodedb_abi_version bump) stand as proposed.

[farhan-syah]

All three follow-ups shipped in b491eba on main.

What changed

• CONTRIBUTING.md — freeze point stated: signatures freeze at the first tagged release.
• nodedb-lite-ffi/abi/surface.txt — every export recorded as a full declaration, C and JNI.
• .github/workflows/test.yml — nm -D diff against that record, plus git diff --exit-code on the generated header.
• nodedb-lite-ffi/src/version.rs — bump rule documented. Adding an export is compatible. Changing or removing one is breaking.
• nodedb-lite-ffi/tests/abi_surface.rs — the record carries abi_version; a test fails when it disagrees with nodedb_abi_version().

The nm -D check runs on every CI run, not only at release. It compares against the committed record, so it works before the first tag — the period the surface changes most.

Full declarations, not names and arity. A return widening from uint32_t to uint64_t keeps the name and the argument count.

Two things the audit found

• Two prefixes in the C API: 27 nodedb_* against 6 ndb_array_*. Renamed to nodedb_array_*. Names freeze with signatures, so that window closes at the first tag.
• 22 JNI exports against 14 Kotlin declarations. nativeGenerateId, nativeGenerateIdTyped and six nativeArray* had no Kotlin side. Declarations and wrappers added; two parity tests hold both sides together.

How to check

cargo nextest run -p nodedb-lite-ffi -E 'binary(abi_surface)'

Re-record after a deliberate surface change:

UPDATE_ABI_SNAPSHOT=1 cargo nextest run -p nodedb-lite-ffi -E 'binary(abi_surface)'

Each drift class was injected and the failure confirmed:

Fault	Caught by
Kotlin extern loses a parameter	kotlin_arity_matches_every_jni_export
Kotlin declaration deleted	kotlin_declares_every_jni_export
nodedb_abi_version widened to u64	surface_matches_snapshot

The first is the crash class from #15.

What none of this catches

Ownership changes keep every signature identical. A returned pointer changing owner breaks every caller silently. docs/ffi-abi.md states the current rules; review is the only guard.

docs/ffi-abi.md is written for adapter authors: the freeze point, the equality check to write against nodedb_abi_version(), and the memory rules. abi/surface.txt ships in the Linux, Android and iOS release artifacts, so an adapter author diffs two releases without cloning.