fix: review follow-ups — idempotency preview guard + TTL cache + empty-array metadata

Addresses the three deferred review comments from #663:

1. Idempotency no longer dedupes SIMULATED contractWrite previews. A preview
   (is_simulated unset/true) skips the idempotency cache entirely, so a client
   reusing a key across a preview and a later real execute can't get the cached
   preview back in place of the broadcast. Only an explicit is_simulated=false
   contractWrite is deduped (contractWriteIsSimulated mirrors the runner default).

2. The idem:noderun: cache now uses a native Badger TTL (new Storage.SetWithTTL,
   backed by badger.Entry.WithTTL) so entries are reaped by the store even when a
   key is never re-read (the common fresh-key-per-action case) — bounding growth
   instead of relying on read-after-expiry deletion. The embedded timestamp +
   read-time check remain as a safety net.

3. runNodeRespToOpenAPI wraps an EMPTY results array as {results: []} instead of
   dropping it to metadata=null, so clients see a consistent shape for a
   legitimately empty result set.

Also: gofmt trailing newline in version/version.go (left by the release bump).

Tests: contractWriteIsSimulated resolution, empty-array wrapping. Build clean.

Author: Chris Li

aggregator/rest/handlers_nodes.go

@@ -137,11 +137,14 @@ func runNodeRespToOpenAPI(in *avsproto.RunNodeWithInputsResp) generated.RunNodeR
 			// results array as metadata. RunNodeResponse.metadata is an object,
 			// so a bare array was previously dropped here — taking the per-method
 			// receipts (executionStatus, userOpHash, transactionHash) with it.
-			// Wrap it under "results" so those actually reach the client.
-			if len(v) > 0 {
-				wrapped := map[string]interface{}{"results": v}
-				out.Metadata = &wrapped
+			// Wrap it under "results" so those reach the client — including an
+			// EMPTY array (results: []), so callers always see a consistent shape
+			// rather than metadata=null for a legitimately empty result set.
+			if v == nil {
+				v = []interface{}{}
 			}
+			wrapped := map[string]interface{}{"results": v}
+			out.Metadata = &wrapped
 		}
 	}
 	if ec := in.GetExecutionContext(); ec != nil {

aggregator/rest/handlers_nodes_test.go

@@ -42,6 +42,21 @@ func TestRunNodeRespToOpenAPISurfacesContractWriteReceiptArray(t *testing.T) {
 	assert.Equal(t, "0xtx", receipt["transactionHash"])
 }
 
+// An EMPTY results array must still surface as {results: []}, not metadata=null,
+// so clients see a consistent shape for a legitimately empty result set.
+func TestRunNodeRespToOpenAPIEmptyArrayMetadataWrapped(t *testing.T) {
+	md, err := structpb.NewValue([]interface{}{})
+	require.NoError(t, err)
+
+	out := runNodeRespToOpenAPI(&avsproto.RunNodeWithInputsResp{Success: true, Metadata: md})
+
+	require.NotNil(t, out.Metadata, "empty array metadata must still be surfaced")
+	m := *out.Metadata
+	arr, ok := m["results"].([]interface{})
+	require.True(t, ok, "empty array must be wrapped under 'results'")
+	assert.Empty(t, arr)
+}
+
 // Map-typed metadata (e.g. ethTransfer's gasUsed/transactionHash) must keep
 // surfacing as-is, without the "results" wrapper.
 func TestRunNodeRespToOpenAPIMapMetadataStillForwarded(t *testing.T) {

core/taskengine/run_node_immediately.go

@@ -3037,13 +3037,35 @@ func idempotencyCacheKey(subject, clientKey string) []byte {
 // a still-pending UserOp) is cached; a transport/engine error caches nothing so
 // the caller may safely retry. Polling a pending UserOp's final status is a
 // separate concern — reuse of the same key returns the cached pending result.
+// contractWriteIsSimulated mirrors the runner's is_simulated resolution for a
+// contractWrite node: the default is SIMULATION unless is_simulated is explicitly
+// set to false. Only contractWrite carries this flag and is the node that
+// broadcasts a real UserOp on this path. Returns false for any other node type so
+// their idempotency behavior is unchanged.
+func contractWriteIsSimulated(node *avsproto.TaskNode) bool {
+	cw := node.GetContractWrite()
+	if cw == nil {
+		return false
+	}
+	cfg := cw.GetConfig()
+	if cfg == nil || cfg.IsSimulated == nil {
+		return true // unset => simulation (matches the runner default)
+	}
+	return cfg.GetIsSimulated()
+}
+
 func (n *Engine) RunNodeImmediatelyRPCIdempotent(ctx context.Context, user *model.User, req *avsproto.RunNodeWithInputsReq, idempotencyKey string) (*avsproto.RunNodeWithInputsResp, error) {
 	// Idempotency needs a distinct authenticated subject to scope the cache key.
 	// A partner-assertion (simulate-only) caller can resolve to the zero address;
 	// deduping those would let one caller's key cross-replay another's result, so
 	// skip idempotency when there is no real subject. Safe by construction: real
 	// executes (fund-moving) always carry a non-zero authenticated owner.
-	if idempotencyKey == "" || user == nil || user.Address == (common.Address{}) {
+	//
+	// Also skip for a SIMULATED contractWrite: idempotency exists to prevent a
+	// second broadcast of a real UserOp, so a preview must never be cached — else a
+	// client that reused a key across a preview and a later real execute could get
+	// the cached preview back in place of the broadcast.
+	if idempotencyKey == "" || user == nil || user.Address == (common.Address{}) || contractWriteIsSimulated(req.GetNode()) {
 		return n.RunNodeImmediatelyRPCWithContext(ctx, user, req)
 	}
 	cacheKey := idempotencyCacheKey(user.Address.Hex(), idempotencyKey)
@@ -3118,7 +3140,10 @@ func (n *Engine) writeIdempotentResponse(cacheKey []byte, resp *avsproto.RunNode
 	buf := make([]byte, 8+len(payload))
 	binary.BigEndian.PutUint64(buf[:8], uint64(time.Now().UnixNano()))
 	copy(buf[8:], payload)
-	if err := n.db.Set(cacheKey, buf); err != nil && n.logger != nil {
+	// Persist with a native store TTL so the entry is reaped by the DB even if it is
+	// never read again (the common case — a fresh key per action). The embedded
+	// timestamp + read-time expiry check remain as a belt-and-suspenders safety net.
+	if err := n.db.SetWithTTL(cacheKey, buf, idempotencyTTL); err != nil && n.logger != nil {
 		n.logger.Warn("RunNodeImmediately: failed to persist idempotent response", "error", err)
 	}
 }

core/taskengine/run_node_immediately_idempotency_test.go

@@ -16,6 +16,28 @@ import (
 	"google.golang.org/protobuf/proto"
 )
 
+// TestContractWriteIsSimulated locks the guard that keeps idempotency off simulated
+// contractWrite previews: default (unset) is simulation, only an explicit
+// is_simulated=false is a real execute, and non-contractWrite nodes are unaffected.
+func TestContractWriteIsSimulated(t *testing.T) {
+	cwNode := func(sim *bool) *avsproto.TaskNode {
+		return &avsproto.TaskNode{TaskType: &avsproto.TaskNode_ContractWrite{
+			ContractWrite: &avsproto.ContractWriteNode{
+				Config: &avsproto.ContractWriteNode_Config{IsSimulated: sim},
+			},
+		}}
+	}
+	tr, fa := true, false
+
+	require.True(t, contractWriteIsSimulated(cwNode(nil)), "unset => simulation (runner default)")
+	require.True(t, contractWriteIsSimulated(cwNode(&tr)), "explicit true => simulated")
+	require.False(t, contractWriteIsSimulated(cwNode(&fa)), "explicit false => real execute")
+	require.False(t, contractWriteIsSimulated(nil), "nil node => not simulated (unchanged)")
+	require.False(t, contractWriteIsSimulated(&avsproto.TaskNode{
+		TaskType: &avsproto.TaskNode_CustomCode{CustomCode: &avsproto.CustomCodeNode{}},
+	}), "non-contractWrite => idempotency behavior unchanged")
+}
+
 // customCodeReq builds a single-node RunNodeWithInputs request whose customCode
 // node returns the given constant, so distinct sources produce distinct results.
 func customCodeReq(source string) *avsproto.RunNodeWithInputsReq {

storage/db.go

@@ -7,6 +7,7 @@ import (
 	"os"
 	"strconv"
 	"strings"
+	"time"
 
 	badger "github.com/dgraph-io/badger/v4"
 )
@@ -56,6 +57,10 @@ type Storage interface {
 	BatchWrite(updates map[string][]byte) error
 	Move(src, dest []byte) error
 	Set(key, value []byte) error
+	// SetWithTTL writes a key that the store auto-expires after `ttl`, so short-lived
+	// entries (e.g. idempotency replay cache) are reaped by the DB itself rather than
+	// only on read-after-expiry — bounding growth for keys that are never re-read.
+	SetWithTTL(key, value []byte, ttl time.Duration) error
 	Delete(key []byte) error
 
 	GetCounter(key []byte, defaultValue ...uint64) (uint64, error)
@@ -140,6 +145,12 @@ func (s *BadgerStorage) Set(key, value []byte) error {
 	})
 }
 
+func (s *BadgerStorage) SetWithTTL(key, value []byte, ttl time.Duration) error {
+	return s.db.Update(func(txn *badger.Txn) error {
+		return txn.SetEntry(badger.NewEntry(key, value).WithTTL(ttl))
+	})
+}
+
 func (s *BadgerStorage) Delete(key []byte) error {
 	return s.db.Update(func(txn *badger.Txn) error {
 		err := txn.Delete(key)

version/version.go

@@ -22,4 +22,4 @@ func Get() string {
 // GetRevision returns the revision
 func GetRevision() string {
 	return revision
-}
+}