fix(error-interception): guard WeakMap accessors against non-object task keys

getTaskState guarded only falsy keys and the module-level
getTaskErrorState/hasTaskErrorState had no guard at all, so a primitive
non-null key (e.g. a string taskId, an easy mistake since
InterceptorOptions.taskId is a string) still threw TypeError on
WeakMap.set(). Both accessors now fail open: invalid keys get an ephemeral
state that is never stored, matching the existing fail-open philosophy.

Author: Zoo (VP)

src/core/tools/error-interception/TaskErrorState.ts

@@ -143,12 +143,28 @@ export class TaskErrorState {
  */
 const taskStates = new WeakMap<object, TaskErrorState>()
 
+/**
+ * Returns true when the argument can be used as a WeakMap key. Primitives
+ * (including string taskIds, an easy mistake) and null/undefined cannot.
+ */
+function isWeakMapKey(task: object): boolean {
+	return !!task && (typeof task === "object" || typeof task === "function")
+}
+
 /**
  * Returns the persistent TaskErrorState for the given Task, creating it on
  * first access. The Task argument is typed as object to keep this module
  * decoupled from the concrete Task class.
+ *
+ * Non-object keys (null/undefined/primitives) fail open with an ephemeral
+ * instance instead of throwing TypeError from WeakMap.set(); ephemeral
+ * instances are never stored, so counters do not persist across calls for
+ * invalid keys.
  */
 export function getTaskErrorState(task: object): TaskErrorState {
+	if (!isWeakMapKey(task)) {
+		return new TaskErrorState()
+	}
 	let state = taskStates.get(task)
 	if (!state) {
 		state = new TaskErrorState()
@@ -160,8 +176,12 @@ export function getTaskErrorState(task: object): TaskErrorState {
 /**
  * Returns true when a TaskErrorState already exists for the given Task,
  * without materializing a new instance. Use this to guard reset paths that
- * must not create empty state as a side effect.
+ * must not create empty state as a side effect. Returns false for keys that
+ * cannot be stored in the WeakMap.
  */
 export function hasTaskErrorState(task: object): boolean {
+	if (!isWeakMapKey(task)) {
+		return false
+	}
 	return taskStates.has(task)
 }

src/core/tools/error-interception/ToolErrorInterceptor.ts

@@ -90,15 +90,16 @@ export class ToolErrorInterceptor {
 	 * Creates or returns existing per-task state. Uses a WeakMap keyed by the
 	 * Task object so state is discarded when the task is garbage collected.
 	 *
-	 * When `task` is null or undefined (invalid WeakMap key), returns an
-	 * ephemeral default state to satisfy the fail-open philosophy rather than
-	 * throwing TypeError from WeakMap.set().
+	 * When `task` is not a valid WeakMap key (null, undefined, or a primitive
+	 * such as a string taskId — an easy mistake since InterceptorOptions.taskId
+	 * is a string), returns an ephemeral default state to satisfy the fail-open
+	 * philosophy rather than throwing TypeError from WeakMap.set().
 	 */
 	public getTaskState(task: object): InterceptorTaskState {
-		// WeakMap keys must be objects; null/undefined are invalid and would
-		// throw TypeError on .set(). Fail-open: return an ephemeral default
-		// state so callers can proceed without crashing.
-		if (!task) {
+		// WeakMap keys must be objects (or functions); primitives are invalid
+		// and would throw TypeError on .set(). Fail-open: return an ephemeral
+		// default state so callers can proceed without crashing.
+		if (!task || (typeof task !== "object" && typeof task !== "function")) {
 			return { categoryCounts: new Map(), shellCircuitOpen: false }
 		}
 		let taskState = this.state.perTask.get(task)

src/core/tools/error-interception/__tests__/TaskErrorState.spec.ts

@@ -169,3 +169,42 @@ describe("hasTaskErrorState", () => {
 		expect(hasTaskErrorState(taskB)).toBe(false)
 	})
 })
+
+describe("non-object key guards", () => {
+	// Double assertions are required below to simulate the caller mistake these
+	// guards protect against: passing a primitive (e.g. a string taskId) or
+	// null/undefined where a Task object is expected. There is no typed way to
+	// express that mistake.
+
+	it("getTaskErrorState returns an ephemeral state for a primitive key instead of throwing", () => {
+		const notATask = "task-id" as unknown as object
+		expect(() => getTaskErrorState(notATask)).not.toThrow()
+		// Ephemeral: nothing is stored in the WeakMap for invalid keys.
+		expect(hasTaskErrorState(notATask)).toBe(false)
+	})
+
+	it("getTaskErrorState returns a fresh ephemeral instance per call for invalid keys", () => {
+		const notATask = "task-id" as unknown as object
+		expect(getTaskErrorState(notATask)).not.toBe(getTaskErrorState(notATask))
+	})
+
+	it("getTaskErrorState tolerates null and undefined keys", () => {
+		expect(() => getTaskErrorState(null as unknown as object)).not.toThrow()
+		expect(() => getTaskErrorState(undefined as unknown as object)).not.toThrow()
+	})
+
+	it("hasTaskErrorState returns false for primitive and nullish keys", () => {
+		expect(hasTaskErrorState("task-id" as unknown as object)).toBe(false)
+		expect(hasTaskErrorState(42 as unknown as object)).toBe(false)
+		expect(hasTaskErrorState(null as unknown as object)).toBe(false)
+		expect(hasTaskErrorState(undefined as unknown as object)).toBe(false)
+	})
+
+	it("still works normally for object keys after guarded calls", () => {
+		const task = { id: "task-after-guard" }
+		getTaskErrorState("task-id" as unknown as object).incrementOccurrence("PARAM_MISSING")
+		expect(getTaskErrorState(task).getOccurrence("PARAM_MISSING")).toBe(0)
+		getTaskErrorState(task).incrementOccurrence("PARAM_MISSING")
+		expect(getTaskErrorState(task).getOccurrence("PARAM_MISSING")).toBe(1)
+	})
+})

src/core/tools/error-interception/__tests__/ToolErrorInterceptor.spec.ts

@@ -607,6 +607,37 @@ describe("ToolErrorInterceptor", () => {
 		})
 	})
 
+	describe("getTaskState non-object key guard", () => {
+		// Double assertions are required below to simulate the caller mistake
+		// this guard protects against: passing a primitive (e.g. the string
+		// InterceptorOptions.taskId) where a Task object is expected. There is
+		// no typed way to express that mistake.
+
+		it("returns an ephemeral state for a string key instead of throwing", () => {
+			const interceptor = createToolErrorInterceptor()
+			const notATask = "task-123" as unknown as object
+			expect(() => interceptor.getTaskState(notATask)).not.toThrow()
+			// Ephemeral: nothing is persisted for invalid keys, so each call
+			// returns a fresh state container.
+			expect(interceptor.getTaskState(notATask)).not.toBe(interceptor.getTaskState(notATask))
+		})
+
+		it("returns an ephemeral state for null, undefined, and numeric keys", () => {
+			const interceptor = createToolErrorInterceptor()
+			expect(() => interceptor.getTaskState(null as unknown as object)).not.toThrow()
+			expect(() => interceptor.getTaskState(undefined as unknown as object)).not.toThrow()
+			expect(() => interceptor.getTaskState(42 as unknown as object)).not.toThrow()
+		})
+
+		it("ephemeral state does not leak into real task state", () => {
+			const interceptor = createToolErrorInterceptor()
+			const notATask = "task-123" as unknown as object
+			interceptor.getTaskState(notATask).categoryCounts.set("SHELL_INTEGRATION", 5)
+			const task = createTask()
+			expect(interceptor.getTaskState(task).categoryCounts.get("SHELL_INTEGRATION")).toBeUndefined()
+		})
+	})
+
 	describe("MCP branch compatibility", () => {
 		it("forwards the feedbackImages second argument unchanged", () => {
 			const interceptor = createToolErrorInterceptor()