fix(warp-core): refuse create-from-absence against an orphaned attachment

Two review findings, same root cause: GraphStore::set_node_attachment
never requires the owning node to exist, so an attachment present
with no NodeRecord (an orphan) is a real reachable state, not
hypothetical. Neither admission nor evaluation independently checked
for it when the node itself was absent.

- current_application_basis (Codex, P2): derived "absent" from node
  presence, so it never inspected the attachment when the node was
  missing. A create-from-absence invocation dishonestly claiming
  absence against a real orphan was admitted. Fixed by deriving
  absence from the attachment slot alone -- the only thing this
  primitive actually mutates -- so an orphan corroborates as present,
  using its real value, exactly like the ordinary case.

- prepare_operation_v1's create branch (CodeRabbit, Major): built the
  atomic create ops without checking whether the attachment slot was
  already occupied, since node absence doesn't imply attachment
  absence. A caller with a basis honestly reflecting an orphan's real
  present value (passing the now-fixed admission check) could still
  reach evaluation and silently overwrite it. Fixed by checking
  store.node_attachment(&node.local_id).is_some() before proceeding,
  refusing with PreconditionMismatch -- creation is atomic over both
  slots or it refuses, per ADR 0024.

New fixture_host_with_orphan_attachment helper backs two tests, one
per layer: admission refuses a dishonest absence claim with
BasisMismatch; evaluation refuses (and leaves the orphan untouched)
when the claimed basis honestly reflects the orphan's real value,
isolating the evaluation-time check specifically. The two fixes had
to land in that order -- before the admission fix, the evaluation
test's premise (an honest "present" basis matching what admission
computes) wasn't reachable, since old admission unconditionally
computed "absent" whenever the node was missing.

22 tests pass (was 20); full pipeline suite green.

Author: flyingrobots

crates/warp-core/src/echo_operation.rs

@@ -2029,20 +2029,25 @@ fn current_application_basis(
             let store = state
                 .store(&invocation.node.warp_id)
                 .ok_or_else(|| basis_mismatch("application-basis warp is unavailable"))?;
-            // A node that does not exist, or that exists without its alpha
-            // attachment, corroborates as absence (ADR 0024): a
-            // create-from-absence invocation's claimed application basis is
-            // then judged, like any other, by the equality check in
-            // `admit_invocation_v1` -- a claim of absence matches only real
-            // absence, exactly as a claim of a value matches only that exact
-            // value. The atomic-or-refuse distinction between "node absent"
-            // and "node exists without its attachment" belongs to
-            // `prepare_operation_v1`, not to this coarse admission check.
-            let attachment = match store.node(&invocation.node.local_id) {
-                None => None,
-                Some(_) => store.node_attachment(&invocation.node.local_id),
-            };
-            let Some(attachment) = attachment else {
+            // Absence corroborates from the attachment slot alone, never from
+            // node presence (ADR 0024, PR #686 review finding #2): a node
+            // that does not exist, or that exists without its alpha
+            // attachment, both corroborate as absence, but an *attachment*
+            // that exists -- even orphaned, with no owning `NodeRecord`,
+            // which `GraphStore::set_node_attachment` never forbids -- must
+            // corroborate as present, using its real value, exactly like the
+            // ordinary case below. Deriving "absent" from node presence
+            // instead of attachment presence would let a claimed-absent
+            // invocation match an orphaned attachment's slot without ever
+            // inspecting it. A create-from-absence invocation's claimed
+            // application basis is then judged, like any other, by the
+            // equality check in `admit_invocation_v1` -- a claim of absence
+            // matches only real absence, exactly as a claim of a value
+            // matches only that exact value. The atomic-or-refuse
+            // distinction between "node absent" and "node exists without its
+            // attachment" belongs to `prepare_operation_v1`, not to this
+            // coarse admission check.
+            let Some(attachment) = store.node_attachment(&invocation.node.local_id) else {
                 // No local budget guard is needed here: `admit_invocation_v1`
                 // already requires `invocation.delegated_budget.read_bytes >=
                 // installed.program.minimum_budget().read_bytes` (64 for this
@@ -2399,6 +2404,16 @@ pub(crate) fn prepare_operation_v1(
                         return obstruction(EchoOperationObstructionKindV1::BudgetExceeded);
                     }
                     actual_footprint.a_read.insert(slot);
+                    // PR #686 review finding #2 (CodeRabbit): the node being
+                    // absent does not prove the attachment slot is absent
+                    // too -- `GraphStore::set_node_attachment` never requires
+                    // the owning node to exist, so an orphaned attachment is
+                    // a real reachable state, not hypothetical. Creation is
+                    // atomic over both slots (ADR 0024); refuse rather than
+                    // silently overwrite an orphan's real value.
+                    if store.node_attachment(&node.local_id).is_some() {
+                        return obstruction(EchoOperationObstructionKindV1::PreconditionMismatch);
+                    }
                     let Some(write_bytes) = 32_u64.checked_add(replacement_len) else {
                         return obstruction(EchoOperationObstructionKindV1::BudgetExceeded);
                     };

crates/warp-core/tests/executable_operation_pipeline_tests.rs

@@ -230,6 +230,84 @@ fn fixture_host_with_bare_node(
     (host, head_key, node, second_node)
 }
 
+/// Like [`fixture_host`], but the warp-scoped store also contains an
+/// *orphaned* alpha attachment -- present at `orphan_local_id` with no
+/// corresponding `NodeRecord` -- reachable because
+/// `GraphStore::set_node_attachment` does not require the owning node to
+/// exist. Used to exercise create-from-absence's PR #686 review finding #2
+/// (CodeRabbit + Codex): a node that is absent must not be treated as fully
+/// absent when its attachment slot is independently occupied.
+fn fixture_host_with_orphan_attachment(
+    orphan_local_id: warp_core::NodeId,
+    orphan_attachment_type: warp_core::TypeId,
+    orphan_bytes: &[u8],
+) -> (TrustedRuntimeHost, WriterHeadKey, NodeKey, NodeKey) {
+    let warp_id = make_warp_id("operation-fixture");
+    let node_id = make_node_id("operation-fixture-root");
+    let node_type = make_type_id("operation-fixture-node");
+    let attachment_type = make_type_id("operation-fixture-atom");
+    let node = NodeKey {
+        warp_id,
+        local_id: node_id,
+    };
+    let orphan_node = NodeKey {
+        warp_id,
+        local_id: orphan_local_id,
+    };
+    let mut store = GraphStore::new(warp_id);
+    store.insert_node(node_id, NodeRecord { ty: node_type });
+    store.set_node_attachment(
+        node_id,
+        Some(AttachmentValue::Atom(AtomPayload::new(
+            attachment_type,
+            Bytes::from_static(b"before"),
+        ))),
+    );
+    store.set_node_attachment(
+        orphan_local_id,
+        Some(AttachmentValue::Atom(AtomPayload::new(
+            orphan_attachment_type,
+            Bytes::copy_from_slice(orphan_bytes),
+        ))),
+    );
+    let state = WorldlineState::from_root_store(store, node_id)
+        .expect("the fixture state has one lawful root");
+    let worldline_id = WorldlineId::from_bytes(digest("operation-fixture-worldline"));
+    let head_key = WriterHeadKey {
+        worldline_id,
+        head_id: make_head_id("operation-fixture-writer"),
+    };
+    let mut runtime = WorldlineRuntime::new();
+    runtime
+        .register_worldline(worldline_id, state)
+        .expect("the fixture worldline registers");
+    runtime
+        .register_writer_head(WriterHead::with_routing(
+            head_key,
+            PlaybackMode::Play,
+            InboxPolicy::AcceptAll,
+            None,
+            true,
+        ))
+        .expect("the fixture writer registers");
+
+    let mut engine_store = GraphStore::default();
+    let engine_root = make_node_id("root");
+    engine_store.insert_node(
+        engine_root,
+        NodeRecord {
+            ty: make_type_id("world"),
+        },
+    );
+    let engine = EngineBuilder::new(engine_store, engine_root)
+        .scheduler(SchedulerKind::Radix)
+        .workers(1)
+        .build();
+    let host = TrustedRuntimeHost::new(runtime, engine)
+        .expect("the trusted Echo runtime host initializes");
+    (host, head_key, node, orphan_node)
+}
+
 fn semantic_closure() -> EchoOperationSemanticClosureV1 {
     EchoOperationSemanticClosureV1::new(
         digest("fixture-edict-source"),
@@ -2156,3 +2234,122 @@ fn filesystem_wal_recovers_a_create_from_absence_commit() {
         )))
     );
 }
+
+/// Regression for PR #686 review finding #2 (CodeRabbit, P0): create-from-
+/// absence must not silently overwrite an orphaned attachment (present with
+/// no owning `NodeRecord` -- reachable because `GraphStore::set_node_attachment`
+/// never requires the node to exist). Exercised with a basis that honestly
+/// reflects the orphan's real present value, so admission passes and the
+/// evaluation-time check in `prepare_operation_v1` is isolated as the thing
+/// under test.
+#[test]
+fn create_from_absence_refuses_when_the_attachment_exists_without_its_node() {
+    let orphan_local_id = make_node_id("operation-fixture-orphan-attachment");
+    let orphan_attachment_type = make_type_id("operation-fixture-atom");
+    let (mut host, head_key, _existing_node, orphan_node) =
+        fixture_host_with_orphan_attachment(orphan_local_id, orphan_attachment_type, b"orphaned");
+    let installed = install_fixture_operation(&mut host);
+
+    let application_basis = warp_core::echo_operation_anchored_node_application_basis_v1(
+        orphan_node,
+        orphan_attachment_type,
+        b"orphaned",
+    );
+    let evaluation_basis = host
+        .echo_operation_evaluation_basis_v1(head_key, application_basis)
+        .expect("Echo resolves the exact current parent basis");
+    let invocation = EchoOperationInvocationV1::anchored_node_attachment_compare_and_set(
+        installed.package_id(),
+        installed.operation_coordinate(),
+        evaluation_basis,
+        digest("fixture-authority-grant"),
+        EchoOperationBudgetV1::new(16, 4_096, 4_096),
+        orphan_node,
+        None,
+        b"clobbered".to_vec(),
+    );
+    let invocation_bytes = invocation
+        .to_canonical_bytes()
+        .expect("the create-from-absence invocation is canonical");
+    let invocation_policy = EchoOperationInvocationAdmissionPolicyV1::new(
+        digest("fixture-authority-profile"),
+        digest("fixture-authority-grant"),
+        EchoOperationBudgetV1::new(16, 4_096, 4_096),
+    );
+    let admitted_invocation = host
+        .admit_echo_operation_invocation_v1(&invocation_policy, &invocation_bytes)
+        .expect(
+            "Echo admits the invocation -- the orphaned attachment honestly \
+             corroborates as present, matching this invocation's claimed basis",
+        );
+    let preparation = host.prepare_echo_operation_v1(admitted_invocation);
+    let EchoOperationPreparationV1::Obstructed(obstruction) = preparation else {
+        panic!("create-from-absence against an orphaned attachment must not prepare a patch");
+    };
+    assert_eq!(
+        obstruction.kind(),
+        EchoOperationObstructionKindV1::PreconditionMismatch
+    );
+
+    let state = host
+        .runtime()
+        .worldlines()
+        .get(&head_key.worldline_id)
+        .expect("the untouched worldline remains registered")
+        .state();
+    assert_eq!(
+        state
+            .store(&orphan_node.warp_id)
+            .and_then(|store| store.node_attachment(&orphan_node.local_id)),
+        Some(&AttachmentValue::Atom(AtomPayload::new(
+            orphan_attachment_type,
+            Bytes::from_static(b"orphaned"),
+        ))),
+        "an obstructed create-from-absence must leave the orphaned attachment untouched"
+    );
+}
+
+/// Regression for PR #686 review finding #2 (Codex, P2): admission's
+/// independent application-basis corroboration must not treat a node with an
+/// orphaned attachment as absent. A create-from-absence invocation that
+/// (incorrectly) claims absence against a real orphan must be refused at
+/// admission with `BasisMismatch`, not let through to evaluation.
+#[test]
+fn create_from_absence_invocation_refuses_at_admission_against_an_orphaned_attachment() {
+    let orphan_local_id = make_node_id("operation-fixture-orphan-admission");
+    let orphan_attachment_type = make_type_id("operation-fixture-atom");
+    let (mut host, head_key, _existing_node, orphan_node) =
+        fixture_host_with_orphan_attachment(orphan_local_id, orphan_attachment_type, b"orphaned");
+    let installed = install_fixture_operation(&mut host);
+
+    let claimed_absent_basis =
+        warp_core::echo_operation_anchored_node_absent_application_basis_v1(orphan_node);
+    let evaluation_basis = host
+        .echo_operation_evaluation_basis_v1(head_key, claimed_absent_basis)
+        .expect("Echo resolves a basis for the claimed proposition");
+    let invocation = EchoOperationInvocationV1::anchored_node_attachment_compare_and_set(
+        installed.package_id(),
+        installed.operation_coordinate(),
+        evaluation_basis,
+        digest("fixture-authority-grant"),
+        EchoOperationBudgetV1::new(16, 4_096, 4_096),
+        orphan_node,
+        None,
+        b"clobbered".to_vec(),
+    );
+    let invocation_bytes = invocation
+        .to_canonical_bytes()
+        .expect("the create-from-absence invocation is canonical");
+    let invocation_policy = EchoOperationInvocationAdmissionPolicyV1::new(
+        digest("fixture-authority-profile"),
+        digest("fixture-authority-grant"),
+        EchoOperationBudgetV1::new(16, 4_096, 4_096),
+    );
+    let error = host
+        .admit_echo_operation_invocation_v1(&invocation_policy, &invocation_bytes)
+        .expect_err("a dishonest absence claim against a real orphaned attachment must refuse");
+    assert_eq!(
+        error.kind(),
+        EchoOperationInvocationAdmissionErrorKindV1::BasisMismatch
+    );
+}