Range reads honor the transaction option: getRange(), getKeys(), and getKeysCount() see a transaction's staged writes and snapshot

AGENTS.md

@@ -34,10 +34,12 @@ GitHub Copilot, and other AI coding assistants when working with code in this re
 - `pnpm type-check` - TypeScript type checking only
 
 **Run `pnpm fmt` before every commit** (or `pnpm check` to also type-check and lint) — CI runs
-`pnpm fmt:check` and fails the build on unformatted code. Note the scope: oxfmt formats **TS/JS/JSON
-only**. It does **not** touch C++ or Markdown, so changes to `src/binding/**` and to docs like this
-file (`AGENTS.md`) are not auto-formatted and must be checked by hand — a mis-numbered invariant or a
-stray C++ indent will pass `fmt:check` untouched.
+`pnpm fmt:check` and fails the build on unformatted code. Note the scope: oxfmt formats TS/JS/JSON
+**and Markdown**, but **not C++**, so a stray indent under `src/binding/**` passes `fmt:check`
+untouched and must be checked by hand. Markdown is checked, which includes the ordered list of
+invariants below: a branch that adds an invariant while `main` adds another **renumbers cleanly in
+git and still fails `fmt:check` on the merge ref**, because both sides claim the same number. Rebase
+onto `main` and renumber before pushing rather than reading the red check as unrelated.
 
 ### Development Workflow
 
@@ -739,8 +741,40 @@ sufficient (env teardown does not honor tsfn acquire counts); see
     written remains frozen across retries, though reapplying the same timestamp is idempotent while
     the transaction remains pending. rocksdb-js does not define record value layouts: a producer
     that copies `getTimestamp()` into record bytes must call `setTimestamp()` first.
-
-20. **A WriteBufferManager stall is a second, entirely separate stall mechanism, and nothing in
+20. **Transactional ranges keep the caller's column family and close before the transaction**:
+    `Store.getRange()` routes `options.transaction` to native by transaction ID, where the caller
+    database descriptor resolves it and supplies the caller's `DBHandle` to `DBIteratorHandle`.
+    Replacing the context with `transaction._context` is incorrect for cross-column-family scans:
+    that native transaction carries the column family on which it was created. Transaction ids are
+    allocated per `DBDescriptor` (`nextTransactionId`), so an id from another database resolves in
+    the caller's descriptor to an unrelated transaction of the same number; `Store.getTxnId()`
+    rejects that by comparing `NativeDatabase.identityPath`, the resolved identity the registry
+    keyed the descriptor on (`resolveIdentityPath`), cached on the `Store` at open. Never compare
+    the path a caller passed to `open()`: it is a spelling, so `data` and `./data` — one database
+    and one id space — would be rejected, while one relative path can name two databases across a
+    `chdir`. Column families of a database share the identity, so cross-column-family reads pass.
+    Transaction-backed iterators establish and pass the transaction snapshot, seek explicitly, and
+    enforce their encoded
+    bounds in `valid()` rather than trusting RocksDB alone: `iterate_lower_bound` is inclusive, so the
+    exclusive lower bound of a reverse range (`exclusiveStart`) has to be applied by the handle when
+    the iterator reaches it, and a transaction's write batch ignored the read-option bounds before
+    RocksDB 8.10.0, so a build linked against an older release (`ROCKSDB_VERSION` / `ROCKSDB_PATH`)
+    checks both bounds on transaction iterators (a compile-time `ROCKSDB_MAJOR`/`ROCKSDB_MINOR`
+    check); the pinned 11.8.1 only pays the reverse `exclusiveStart` compare, like a plain iterator.
+    `closeIterators()` waits for a handle that is mid-destruction on another thread to reset its
+    RocksDB iterator before the transaction is freed; it does not serialize a cross-environment
+    close against a `next()` in flight on the owning thread (the descriptor's closables sweep never
+    did either). They register weakly with `TransactionHandle`; commit,
+    abort, the coordinated-retry reset (`resetTransaction`), and forced teardown close every
+    registered iterator before committing, rolling back, resetting, or deleting the RocksDB
+    transaction, so a later `next()` deterministically reports an uninitialized iterator rather than
+    reading freed write-batch state (`return()`/`throw()` stay idempotent so loop cleanup after that
+    close cannot throw), and `createIterator` rejects a range or count once the transaction is no
+    longer pending. The reverse seek always steps off a key equal to the encoded end
+    bound: `inclusiveEnd` appends a NUL to that bound, so the bound itself is exclusive in both
+    directions and a staged key that lands exactly on it must be excluded like a committed one.
+
+21. **A WriteBufferManager stall is a second, entirely separate stall mechanism, and nothing in
     RocksDB reports it**: `DBImpl::WriteBufferManagerStallWrites` parks writers on the manager's own
     queue (`WBMStallInterface::Block`) without touching the `WriteController`, so `rocksdb.stall.micros`,
     the `WRITE_STALL` histogram, `OnStallConditionsChanged` — and therefore the `'writeStall'` event

README.md

@@ -884,6 +884,23 @@ for (const { key, value } of db.getRange({ start: 'a', end: 'z' })) {
 }
 ```
 
+Pass `transaction: txn` to iterate this column family through that transaction, exactly as
+`get()` does with the same option: the iterator sees the transaction's staged writes and reads on its
+snapshot, and range bounds apply to staged keys too. `getKeys()` and `getKeysCount()` accept it as
+well. On a transaction (`txn.getRange()`) the transaction itself is the context and takes precedence
+over a `transaction` option, as it does for `txn.get()`. Such an iterator is closed when the
+transaction commits or aborts: a later `next()` throws, while `return()` stays a no-op. Opening a
+range or counting through a transaction that has already started committing throws as well.
+
+```typescript
+await db.transaction(async (txn) => {
+	await txn.put('c', 'staged');
+	for (const { key, value } of db.getRange({ start: 'a', end: 'z', transaction: txn })) {
+		console.log({ key, value }); // includes { key: "c", value: "staged" }
+	}
+});
+```
+
 ### `db.getUserSharedBuffer(key: Key, defaultBuffer: ArrayBuffer, options?)`
 
 Creates a new buffer with the contents of `defaultBuffer` that can be accessed across threads. This
@@ -2574,7 +2591,9 @@ Options for `get()`, `getSync()`, and the `getBinary*` methods.
   - `tailing: boolean` When `true`, creates a "tailing iterator" which is a special iterator that
     has a view of the complete database including newly added data and is optimized for sequential
     reads. This will return records that were inserted into the database after the creation of the
-    iterator. Defaults to `false`.
+    iterator. Defaults to `false`. A tailing iterator ignores a transaction's snapshot: through a
+    transaction it still reads the latest committed state, merged with that transaction's staged
+    writes.
 
 ### `RangeOptions`
 

src/binding/binding.cpp

@@ -349,6 +349,7 @@ NAPI_MODULE_INIT() {
 	EXPORT_CONSTANT(constants, ITERATOR_INCLUDE_VALUES_FLAG)
 	EXPORT_CONSTANT(constants, ITERATOR_NEEDS_STABLE_VALUE_BUFFER_FLAG)
 	EXPORT_CONSTANT(constants, ITERATOR_CONTEXT_IS_TRANSACTION_FLAG)
+	EXPORT_CONSTANT(constants, ITERATOR_HAS_TRANSACTION_ID_FLAG)
 	EXPORT_CONSTANT(constants, ITERATOR_RESULT_DONE)
 	EXPORT_CONSTANT(constants, ITERATOR_RESULT_FAST)
 	NAPI_STATUS_THROWS(::napi_set_named_property(env, exports, "constants", constants));

src/binding/database/database.cpp

@@ -237,6 +237,29 @@ napi_value Database::Columns(napi_env env, napi_callback_info info) {
 	return result;
 }
 
+/**
+ * The database's resolved filesystem identity — the registry key that two
+ * spellings of one directory (`data` and `./data`, a symlink and its target)
+ * share and that a repointed symlink or a `chdir` cannot change afterwards.
+ * `undefined` until the handle has been opened; retained after close.
+ *
+ * Callers comparing two handles for "same database" must use this and never
+ * the path they passed to `open()`, which is a spelling, not an identity.
+ */
+napi_value Database::IdentityPath(napi_env env, napi_callback_info info) {
+	NAPI_METHOD();
+	UNWRAP_DB_HANDLE();
+
+	if (dbHandle == nullptr || (*dbHandle)->identityPath.empty()) {
+		NAPI_RETURN_UNDEFINED();
+	}
+
+	const std::string& identityPath = (*dbHandle)->identityPath;
+	napi_value result;
+	NAPI_STATUS_THROWS(::napi_create_string_utf8(env, identityPath.c_str(), identityPath.size(), &result));
+	return result;
+}
+
 /**
  * Compacts the entire key range of the database asynchronously.
  * This triggers manual compaction which removes tombstones and reclaims space.
@@ -1327,12 +1350,22 @@ napi_value Database::GetCount(napi_env env, napi_callback_info info) {
 			::napi_throw_error(env, nullptr, errorMsg.c_str());
 			NAPI_RETURN_UNDEFINED();
 		}
-		txnHandle->getCount(itOptions, count, *dbHandle);
+		try {
+			txnHandle->getCount(itOptions, count, *dbHandle);
+		} catch (const std::exception& e) {
+			::napi_throw_error(env, nullptr, e.what());
+			NAPI_RETURN_UNDEFINED();
+		}
 	} else {
-		std::unique_ptr<DBIteratorHandle> itHandle = std::make_unique<DBIteratorHandle>(*dbHandle, itOptions);
-		while (itHandle->iterator->Valid()) {
-			++count;
-			itHandle->iterator->Next();
+		try {
+			std::unique_ptr<DBIteratorHandle> itHandle = std::make_unique<DBIteratorHandle>(*dbHandle, itOptions);
+			while (itHandle->valid()) {
+				++count;
+				itHandle->advance();
+			}
+		} catch (const std::exception& e) {
+			::napi_throw_error(env, nullptr, e.what());
+			NAPI_RETURN_UNDEFINED();
 		}
 	}
 
@@ -2726,6 +2759,7 @@ void Database::Init(napi_env env, napi_value exports) {
 		{ "getSync", nullptr, GetSync, nullptr, nullptr, nullptr, napi_default, nullptr },
 		{ "getUserSharedBuffer", nullptr, GetUserSharedBuffer, nullptr, nullptr, nullptr, napi_default, nullptr },
 		{ "hasLock", nullptr, HasLock, nullptr, nullptr, nullptr, napi_default, nullptr },
+		{ "identityPath", nullptr, nullptr, IdentityPath, nullptr, nullptr, napi_default, nullptr },
 		{ "listeners", nullptr, Listeners, nullptr, nullptr, nullptr, napi_default, nullptr },
 		{ "listLogs", nullptr, ListLogs, nullptr, nullptr, nullptr, napi_default, nullptr },
 		{ "notify", nullptr, Notify, nullptr, nullptr, nullptr, napi_default, nullptr },

src/binding/database/database.h

@@ -329,6 +329,7 @@ struct Database final {
 	static napi_value GetSync(napi_env env, napi_callback_info info);
 	static napi_value GetUserSharedBuffer(napi_env env, napi_callback_info info);
 	static napi_value HasLock(napi_env env, napi_callback_info info);
+	static napi_value IdentityPath(napi_env env, napi_callback_info info);
 	static napi_value IsOpen(napi_env env, napi_callback_info info);
 	static napi_value Listeners(napi_env env, napi_callback_info info);
 	static napi_value ListLogs(napi_env env, napi_callback_info info);

src/binding/iterator/db_iterator.cpp

@@ -76,9 +76,10 @@ napi_status DBIteratorOptions::initFromNapiObject(napi_env env, napi_value optio
  *   argv[4] - endKeyEnd (uint32) - end position of end key; if equal to
  *             endKeyStart there is no end key
  *   argv[5] - optional advanced ReadOptions object (rare path)
+ *   argv[6] - optional transaction ID for a Database context
  */
 napi_value DBIterator::Constructor(napi_env env, napi_callback_info info) {
-	NAPI_CONSTRUCTOR_ARGV("Iterator", 6);
+	NAPI_CONSTRUCTOR_ARGV("Iterator", 7);
 
 	uint32_t flags = 0;
 	NAPI_STATUS_THROWS(::napi_get_value_uint32(env, argv[1], &flags));
@@ -117,6 +118,7 @@ napi_value DBIterator::Constructor(napi_env env, napi_callback_info info) {
 	// napi_get_reference_value + napi_get_named_property + napi_instanceof
 	// per iterator construction.
 	const bool isTransaction = (flags & ITERATOR_CONTEXT_IS_TRANSACTION_FLAG) != 0;
+	const bool hasTransactionId = (flags & ITERATOR_HAS_TRANSACTION_ID_FLAG) != 0;
 
 	std::shared_ptr<DBIteratorHandle>* itHandle = nullptr;
 	std::shared_ptr<DBHandle>* dbHandle = nullptr;
@@ -130,6 +132,14 @@ napi_value DBIterator::Constructor(napi_env env, napi_callback_info info) {
 			return nullptr;
 		}
 		txnHandle = *wrappedTxnHandle;
+		if (!txnHandle->dbHandle) {
+			::napi_throw_error(env, nullptr, "Transaction is not in pending state");
+			return nullptr;
+		}
+		if (!txnHandle->dbHandle->opened()) {
+			::napi_throw_error(env, nullptr, "Database not open");
+			return nullptr;
+		}
 		dbHandle = &txnHandle->dbHandle;
 		DEBUG_LOG("DBIterator::Constructor txnHandle=%p dbHandle=%p\n", txnHandle.get(), dbHandle->get());
 	} else {
@@ -139,6 +149,21 @@ napi_value DBIterator::Constructor(napi_env env, napi_callback_info info) {
 			return nullptr;
 		}
 		DEBUG_LOG("DBIterator::Constructor Initializing iterator handle with Database instance (dbHandle=%p)\n", (*dbHandle).get());
+
+		if (hasTransactionId) {
+			if (argc <= 6) {
+				::napi_throw_type_error(env, nullptr, "Invalid transaction");
+				return nullptr;
+			}
+			uint32_t txnId = 0;
+			NAPI_STATUS_THROWS(::napi_get_value_uint32(env, argv[6], &txnId));
+			txnHandle = (*dbHandle)->descriptor->transactionGet(txnId);
+			if (!txnHandle) {
+				std::string errorMsg = "Iterator failed: Transaction not found (txnId: " + std::to_string(txnId) + ")";
+				::napi_throw_error(env, nullptr, errorMsg.c_str());
+				return nullptr;
+			}
+		}
 	}
 
 	// Resolve start/end key pointers from the shared default key buffer
@@ -163,11 +188,13 @@ napi_value DBIterator::Constructor(napi_env env, napi_callback_info info) {
 	}
 
 	try {
+		std::shared_ptr<DBIteratorHandle> iteratorHandle;
 		if (txnHandle) {
-			itHandle = new std::shared_ptr<DBIteratorHandle>(std::make_shared<DBIteratorHandle>(txnHandle, itOptions));
+			iteratorHandle = txnHandle->createIterator(itOptions, isTransaction ? nullptr : *dbHandle);
 		} else {
-			itHandle = new std::shared_ptr<DBIteratorHandle>(std::make_shared<DBIteratorHandle>(*dbHandle, itOptions));
+			iteratorHandle = std::make_shared<DBIteratorHandle>(*dbHandle, itOptions);
 		}
+		itHandle = new std::shared_ptr<DBIteratorHandle>(std::move(iteratorHandle));
 	} catch (const std::exception& e) {
 		::napi_throw_error(env, nullptr, e.what());
 		return nullptr;
@@ -218,6 +245,16 @@ napi_value DBIterator::Constructor(napi_env env, napi_callback_info info) {
 		} \
 	} while (0)
 
+#define CLOSE_ITERATOR_HANDLE(fnName) \
+	do { \
+		std::shared_ptr<DBIteratorHandle>* itHandle = nullptr; \
+		NAPI_STATUS_THROWS(::napi_unwrap(env, jsThis, reinterpret_cast<void**>(&itHandle))); \
+		if (itHandle && *itHandle) { \
+			DEBUG_LOG("%p DBIterator::" fnName " Closing iterator handle\n", (*itHandle).get()); \
+			(*itHandle)->close(); \
+		} \
+	} while (0)
+
 /**
  * Builds a slow-path object `{ key: Buffer, value?: Buffer }` for the rare case
  * where the shared key/value buffers cannot be used (oversized data or stable
@@ -266,8 +303,8 @@ napi_value DBIterator::Next(napi_env env, napi_callback_info info) {
 	auto& it = *itHandle;
 	napi_value result;
 
-	if (!it->iterator->Valid()) {
-		if (!it->iterator->status().ok()) {
+	if (!it->valid()) {
+		if (it->iterator && !it->iterator->status().ok()) {
 			DEBUG_LOG("%p DBIterator::Next iterator not valid/ok: %s\n", itHandle, it->iterator->status().ToString().c_str());
 		} else {
 			DEBUG_LOG("%p DBIterator::Next iterator no keys found in range\n", it.get());
@@ -278,21 +315,6 @@ napi_value DBIterator::Next(napi_env env, napi_callback_info info) {
 
 	rocksdb::Slice keySlice = it->iterator->key();
 
-	// Edge case: reverse + exclusiveStart and we just landed on the start key
-	// (which is the lower bound for reverse iteration). We need to peek ahead
-	// to determine whether this is the last item.
-	if (it->reverse && it->exclusiveStart &&
-	    it->startKey.size() > 0 && keySlice.compare(it->startKey) == 0) {
-		it->iterator->Prev();
-		if (!it->iterator->Valid()) {
-			NAPI_STATUS_THROWS(::napi_create_uint32(env, ITERATOR_RESULT_DONE, &result));
-			return result;
-		}
-		// not the last item; restore position and continue normally below
-		it->iterator->Next();
-		keySlice = it->iterator->key();
-	}
-
 	// Try the fast path: copy key (and optionally value) into the shared
 	// default buffers and write lengths to the iterator state buffer.
 	auto& dbHandle = it->dbHandle;
@@ -321,22 +343,14 @@ napi_value DBIterator::Next(napi_env env, napi_callback_info info) {
 			::memcpy(valueBuffer, valueSlice.data(), valueSlice.size());
 			state[1] = static_cast<uint32_t>(valueSlice.size());
 		}
-		if (it->reverse) {
-			it->iterator->Prev();
-		} else {
-			it->iterator->Next();
-		}
+		it->advance();
 		NAPI_STATUS_THROWS(::napi_create_uint32(env, ITERATOR_RESULT_FAST, &result));
 		return result;
 	}
 
 	// Slow path: at least one of key or value can't go in the shared buffer.
 	napi_value slowResult = buildSlowResult(env, keySlice, it->values, valueSlice);
-	if (it->reverse) {
-		it->iterator->Prev();
-	} else {
-		it->iterator->Next();
-	}
+	it->advance();
 	return slowResult;
 }
 
@@ -347,11 +361,7 @@ napi_value DBIterator::Next(napi_env env, napi_callback_info info) {
  */
 napi_value DBIterator::Return(napi_env env, napi_callback_info info) {
 	NAPI_METHOD();
-	UNWRAP_ITERATOR_HANDLE("Return");
-
-	DEBUG_LOG("%p DBIterator::Return Closing iterator handle\n", (*itHandle).get());
-	(*itHandle)->close();
-
+	CLOSE_ITERATOR_HANDLE("Return");
 	NAPI_RETURN_UNDEFINED();
 }
 
@@ -361,11 +371,7 @@ napi_value DBIterator::Return(napi_env env, napi_callback_info info) {
  */
 napi_value DBIterator::Throw(napi_env env, napi_callback_info info) {
 	NAPI_METHOD();
-	UNWRAP_ITERATOR_HANDLE("Throw");
-
-	DEBUG_LOG("%p DBIterator::Throw Closing iterator handle\n", (*itHandle).get());
-	(*itHandle)->close();
-
+	CLOSE_ITERATOR_HANDLE("Throw");
 	NAPI_RETURN_UNDEFINED();
 }
 

src/binding/iterator/db_iterator.h

@@ -19,6 +19,7 @@ namespace rocksdb_js {
 // constructor relies on this flag to skip the expensive
 // napi_get_named_property + napi_instanceof type checks.
 #define ITERATOR_CONTEXT_IS_TRANSACTION_FLAG     0x20
+#define ITERATOR_HAS_TRANSACTION_ID_FLAG         0x40
 
 // Iterator Next() return signals
 #define ITERATOR_RESULT_DONE 0