From 1a9a9ec67eb0934e960e3664fb90ecc428bd9018 Mon Sep 17 00:00:00 2001
From: harshitsinghbhandari <24b4506@iitb.ac.in>
Date: Sun, 31 May 2026 18:54:55 +0530
Subject: [PATCH] test(integration): check ok/err before patching session row
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Greptile P1: the patch-then-Update path in TestRestoreRoundTrip was
discarding GetSession's ok/err. A missed row would have handed UpdateSession
a zero-value SessionRecord (ID==""), which matches zero rows and returns
nil — Phase B then fails with the misleading "agent session id lost across
restart" instead of the real cause.

Fixed at the patch site (the only write path that could swallow the error)
and at the two read-then-assert sites in TestDetectingPersistsAcrossRestart
for consistency. Downstream assertions there would already fail loudly, but
the explicit ok/err check makes the failure mode unambiguous.

All 219 tests still pass under -race.
---
 .../integration/lifecycle_sqlite_test.go      | 20 +++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/backend/internal/integration/lifecycle_sqlite_test.go b/backend/internal/integration/lifecycle_sqlite_test.go
index f2f75a71d..477455080 100644
--- a/backend/internal/integration/lifecycle_sqlite_test.go
+++ b/backend/internal/integration/lifecycle_sqlite_test.go
@@ -382,8 +382,14 @@ func TestRestoreRoundTrip_PreservesMetadata(t *testing.T) {
 	// fold an AgentSessionID into the row — the LCM does this through the spawn
 	// outcome on Restore too, but a fresh spawn doesn't (the agent has not
 	// reported one yet). We patch via the store so the restore branch has
-	// something to resume from.
-	rec, _, _ := st.store.GetSession(ctx, sess.ID)
+	// something to resume from. Check ok/err: without it, a missed row would
+	// hand UpdateSession a zero-value record (ID==""), which matches no rows
+	// and returns nil — Phase B would then fail with a misleading "agent id
+	// lost across restart" rather than the real cause.
+	rec, ok, err := st.store.GetSession(ctx, sess.ID)
+	if err != nil || !ok {
+		t.Fatalf("get session for patch: ok=%v err=%v", ok, err)
+	}
 	rec.Metadata.AgentSessionID = "agent-xyz"
 	if err := st.store.UpdateSession(ctx, rec); err != nil {
 		t.Fatalf("patch agent id: %v", err)
@@ -534,7 +540,10 @@ func TestDetectingPersistsAcrossRestart(t *testing.T) {
 	}); err != nil {
 		t.Fatalf("apply runtime: %v", err)
 	}
-	rec, _, _ := st.store.GetSession(ctx, sess.ID)
+	rec, ok, err := st.store.GetSession(ctx, sess.ID)
+	if err != nil || !ok {
+		t.Fatalf("get session post-probe: ok=%v err=%v", ok, err)
+	}
 	if rec.Lifecycle.Session.State != domain.SessionDetecting {
 		t.Fatalf("expected detecting state, got %q", rec.Lifecycle.Session.State)
 	}
@@ -567,7 +576,10 @@ func TestDetectingPersistsAcrossRestart(t *testing.T) {
 	}); err != nil {
 		t.Fatalf("recovery probe: %v", err)
 	}
-	rec3, _, _ := st2.store.GetSession(ctx, sess.ID)
+	rec3, ok3, err := st2.store.GetSession(ctx, sess.ID)
+	if err != nil || !ok3 {
+		t.Fatalf("get session post-recovery: ok=%v err=%v", ok3, err)
+	}
 	if rec3.Lifecycle.Detecting != nil {
 		t.Fatalf("alive probe should clear detecting, got %+v", rec3.Lifecycle.Detecting)
 	}