fix: D17 hardening — fix always-true hasPiFallbackToolOwnerTags gate + 3 tests

Follow-up to the D17 Pi tool-tag owner cutover (5fd440d). The post-merge
adversarial diff-Oracle (SHIP, 0 correctness defects) flagged non-blocking test
gaps; adding the cheap-gate coverage test EXPOSED a real bug it had missed:

hasPiFallbackToolOwnerTags returned `row !== undefined`, but SQLite `.get()`
yields NULL (not the JS undefined sentinel) for a no-row result — so the gate was
ALWAYS true. Correctness was unaffected (findPiFallbackToolOwnerTags returns the
real, empty rows), but the MF-D perf guard was fully defeated: the
buildPiToolOwnerMap branch-walk (resolveStableId over every message) + the
tool-tag scan ran on EVERY Pi pass for EVERY session — exactly the O(session)
hot-path cost MF-D existed to eliminate. Fixed to `!= null`.

Hardened the sibling same-class instance tagTokenCountIsNull, where the bug was
latent-worse: `row !== undefined && row.token_count` would have crashed on
`null.token_count` if ever called for a missing tag.

Added the 3 hardening tests the Oracle recommended: duplicate-only pending-op
retarget onto a survivor lacking the op, no-timestamp owner skip, and the
cheap-gate assertion (which is what surfaced the bug). The cheap-gate test now
also asserts hasPiFallbackToolOwnerTags(...) === false directly.

Gate: plugin 2204/0, Pi 487/0 (+3 tests), tsc + biome clean.

Author: ualtinok

packages/pi-plugin/src/context-handler.test.ts

@@ -18,6 +18,7 @@ import {
 	getPendingOps,
 	getPendingPiCompactionMarkerState,
 	getTagsBySession,
+	hasPiFallbackToolOwnerTags,
 	incrementHistorianFailure,
 	insertTag,
 	queuePendingOp,
@@ -712,6 +713,119 @@ describe("Pi fallback tag adoption", () => {
 			closeQuietly(db);
 		}
 	});
+
+	it("retargets a duplicate-only pending op onto a survivor that lacks it", () => {
+		const db = createTestDb();
+		try {
+			const sessionId = "ses-pi-tool-owner-pending-only-dup";
+			const tagger = createTagger();
+			const callId = "call-pending-only";
+			const piOwner = "pi-msg-0-90-assistant";
+			const realOwner = "entry-tool-pending-only";
+			// Survivor (synthetic) has NO pending op; the duplicate (real re-read) does.
+			insertTag(db, sessionId, callId, "tool", 10, 90, 0, "Read", 0, piOwner);
+			insertTag(db, sessionId, callId, "tool", 20, 91, 0, "Read", 0, realOwner);
+			queuePendingOp(db, sessionId, 91, "drop", 110);
+			tagger.bindToolTag(sessionId, callId, piOwner, 90);
+			tagger.bindToolTag(sessionId, callId, realOwner, 91);
+
+			contextHandlerInternals.adoptPiFallbackTags(
+				db,
+				sessionId,
+				tagger,
+				new Map(),
+				{
+					messages: [assistantToolCall(callId, "Read", {}, 90)],
+					resolveStableId: () => realOwner,
+				},
+			);
+
+			// The duplicate is gone and its pending op moved onto the survivor (90),
+			// which had none — proving retarget, not silent loss.
+			expect(readTagRow(db, sessionId, 91)).toBeUndefined();
+			expect(readTagRow(db, sessionId, 90)?.status).toBe("active");
+			expect(getPendingOps(db, sessionId).map((op) => op.tagId)).toEqual([90]);
+		} finally {
+			closeQuietly(db);
+		}
+	});
+
+	it("skips a no-timestamp fallback owner instead of rekeying it", () => {
+		const db = createTestDb();
+		try {
+			const sessionId = "ses-pi-tool-owner-no-ts";
+			const tagger = createTagger();
+			const callId = "call-no-ts";
+			// No-timestamp synthetic owner form: pi-msg-${index}-${role} (no ts segment).
+			const piOwner = "pi-msg-0-assistant";
+			insertTag(db, sessionId, callId, "tool", 10, 95, 0, "Read", 0, piOwner);
+			tagger.bindToolTag(sessionId, callId, piOwner, 95);
+
+			contextHandlerInternals.adoptPiFallbackTags(
+				db,
+				sessionId,
+				tagger,
+				new Map(),
+				{
+					messages: [assistantToolCall(callId, "Read", {}, 90)],
+					resolveStableId: () => "entry-no-ts",
+				},
+			);
+
+			// Unmatchable by (ts,callID) → left as-is, never wrong-rekeyed.
+			expect(readTagRow(db, sessionId, 95)?.toolOwnerMessageId).toBe(piOwner);
+			expect(
+				tagger.getToolTag(sessionId, callId, "entry-no-ts"),
+			).toBeUndefined();
+		} finally {
+			closeQuietly(db);
+		}
+	});
+
+	it("cheap-gate: does not build the owner map when no pi-msg tool owners exist", () => {
+		const db = createTestDb();
+		try {
+			const sessionId = "ses-pi-tool-owner-cheap-gate";
+			const tagger = createTagger();
+			// Only a real-owner tool tag exists — no pi-msg-* owners to migrate.
+			insertTag(
+				db,
+				sessionId,
+				"call-real",
+				"tool",
+				10,
+				96,
+				0,
+				"Read",
+				0,
+				"entry-real",
+			);
+			// Split a gate hole from a wrong test premise: the tool-owner gate MUST
+			// be false here (no pi-msg-* owners), so the branch-walk never runs.
+			expect(hasPiFallbackToolOwnerTags(db, sessionId)).toBe(false);
+
+			let resolverCalls = 0;
+			contextHandlerInternals.adoptPiFallbackTags(
+				db,
+				sessionId,
+				tagger,
+				new Map(),
+				{
+					messages: [assistantToolCall("call-real", "Read", {}, 90)],
+					resolveStableId: () => {
+						resolverCalls += 1;
+						return "entry-real";
+					},
+				},
+			);
+
+			// The cheap hasPiFallbackToolOwnerTags gate short-circuits before any
+			// branch walk, so the resolver is never consulted.
+			expect(resolverCalls).toBe(0);
+		} finally {
+			closeQuietly(db);
+		}
+	});
 });
 
 describe("registerPiContextHandler", () => {

packages/plugin/src/features/magic-context/storage-tags.ts

@@ -475,8 +475,11 @@ export function updateTagInputTokenCount(
 export function tagTokenCountIsNull(db: Database, sessionId: string, tagNumber: number): boolean {
     const row = db
         .prepare("SELECT token_count FROM tags WHERE session_id = ? AND tag_number = ?")
-        .get(sessionId, tagNumber) as { token_count: number | null } | undefined;
-    return row !== undefined && row.token_count === null;
+        .get(sessionId, tagNumber) as { token_count: number | null } | undefined | null;
+    // `!= null` guards a no-row result (`.get()` yields null, not the JS undefined
+    // sentinel) — `row !== undefined` alone would let that null through to
+    // `null.token_count` and crash.
+    return row != null && row.token_count === null;
 }
 
 /**
@@ -1077,7 +1080,12 @@ export function hasPiFallbackToolOwnerTags(db: Database, sessionId: string): boo
              LIMIT 1`,
         )
         .get(sessionId);
-    return row !== undefined;
+    // `.get()` returns NULL for a no-row result (empirically true under bun:sqlite;
+    // node:sqlite likewise never returns the JS `undefined` sentinel) — so
+    // `row !== undefined` is TRUE for an EMPTY result, which would defeat this cheap
+    // pre-gate and run the per-pass tool-owner branch-walk for EVERY session. Use
+    // `!= null` to treat both null and undefined as "no row".
+    return row != null;
 }
 
 export function findPiFallbackToolOwnerTags(