From 241d79379c2b152a6c4d9cc8878c67953161bf1a Mon Sep 17 00:00:00 2001
From: krandder <azsantos.k@gmail.com>
Date: Tue, 17 Mar 2026 18:38:24 +0000
Subject: [PATCH] Safety fixes: parent-done guard, verification nullsafe,
 metadata inheritance, role leak

1. Parent-done guard (verifyCompletion):
   - Layer 1: block marking parent done if any children are non-terminal
   - Layer 2: block if completionRule=and and not all terminal children are done
   - Layer 3: block if metadata.repo set but stateRef missing/zeroed

2. Role_mismatch identity leak: remove agent ID and assignee/reviewer names
   from error messages to prevent agents from learning who to impersonate

3. Metadata inheritance in decompose handlers: children now inherit all
   parent metadata (minus bookkeeping fields like claimedAt, claimedBy, etc.)
   instead of only priority/assignee/reviewer

4. applyLeaseGranted ts fallback: use Date.now() when event.ts is missing
   to prevent leaseExpiresAt = NaN from external tool events

5. Type fixes (pre-existing compilation errors):
   - Add "file" to ArtifactKind union + path/sizeBytes to ArtifactEvidence
   - Add optional source to TaskCompleted and TaskFailed interfaces
---
 core/reducer.ts |  5 ++-
 core/types.ts   |  6 ++-
 middle/http.ts  | 98 ++++++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 100 insertions(+), 9 deletions(-)

diff --git a/core/reducer.ts b/core/reducer.ts
index 766d128..179165c 100644
--- a/core/reducer.ts
+++ b/core/reducer.ts
@@ -262,10 +262,11 @@ function applyTaskCreated(state: SystemState, event: Extract<Event, { type: "Tas
 }
 
 function applyLeaseGranted(state: SystemState, event: Extract<Event, { type: "LeaseGranted" }>, task: Task): void {
+  const ts = event.ts ?? Date.now();
   const t = deepCloneTask(task);
   t.currentFenceToken = event.fenceToken;
   t.leasedTo = event.agentId;
-  t.leaseExpiresAt = event.ts + event.leaseTimeout;
+  t.leaseExpiresAt = ts + event.leaseTimeout;
   t.phase = event.phase;
   t.condition = "active";
   t.retryAfter = null;
@@ -273,7 +274,7 @@ function applyLeaseGranted(state: SystemState, event: Extract<Event, { type: "Le
   t.currentSessionId = event.agentContext.sessionId;
   t.contextBudget = event.contextBudget;
   t.attempts[event.phase].used += 1;
-  t.updatedAt = event.ts;
+  t.updatedAt = ts;
   state.tasks[t.id] = t;
 }
 
diff --git a/core/types.ts b/core/types.ts
index 97ba289..e55e697 100644
--- a/core/types.ts
+++ b/core/types.ts
@@ -462,7 +462,7 @@ export interface ReviewPolicyMet extends BaseEvent {
 // Completion verification
 // ---------------------------------------------------------------------------
 
-export type ArtifactKind = "journal" | "code" | "pr";
+export type ArtifactKind = "journal" | "code" | "pr" | "file";
 
 export interface ArtifactEvidence {
   kind: ArtifactKind;
@@ -473,6 +473,8 @@ export interface ArtifactEvidence {
   aheadCount?: number | null;
   changedFiles?: string[];
   prUrl?: string | null;
+  path?: string;
+  sizeBytes?: number;
 }
 
 export interface CompletionVerification {
@@ -490,6 +492,7 @@ export interface CompletionVerificationRecorded extends BaseEvent {
 export interface TaskCompleted extends BaseEvent {
   type: "TaskCompleted";
   stateRef: StateRef;
+  source?: EventSource;
 }
 
 export interface TaskFailed extends BaseEvent {
@@ -497,6 +500,7 @@ export interface TaskFailed extends BaseEvent {
   reason: "budget_exhausted" | "cost_exhausted" | "review_rejected";
   phase: Phase;
   summary: FailureSummary;
+  source?: EventSource;
 }
 
 export interface TaskExhausted extends BaseEvent {
diff --git a/middle/http.ts b/middle/http.ts
index 2278a5f..da8cbbf 100644
--- a/middle/http.ts
+++ b/middle/http.ts
@@ -745,12 +745,12 @@ function handleClaimTask(
       if (task.phase === "review") {
         const reviewer = task.metadata["reviewer"] as string | undefined;
         if (reviewer && agentRole(reviewer) !== claimRole) {
-          return { status: 403, body: { error: "role_mismatch", message: `Task ${taskId} reviewer is "${reviewer}", not "${agentId}". Use --force to override.` } };
+          return { status: 403, body: { error: "role_mismatch", message: `Task ${taskId} is assigned to a different agent for review. Use --force to override.` } };
         }
       } else {
         const assignee = task.metadata["assignee"] as string | undefined;
         if (assignee && agentRole(assignee) !== claimRole) {
-          return { status: 403, body: { error: "role_mismatch", message: `Task ${taskId} assignee is "${assignee}", not "${agentId}". Use --force to override.` } };
+          return { status: 403, body: { error: "role_mismatch", message: `Task ${taskId} is assigned to a different agent. Use --force to override.` } };
         }
       }
     }
@@ -1722,6 +1722,74 @@ function maybeCreatePr(config: Config, task: Task, core: Core): void {
   }
 }
 
+function isZeroedStateRef(ref: StateRef): boolean {
+  return ref.commit === "0000000" && ref.parentCommit === "0000000";
+}
+
+/**
+ * Pre-flight checks before marking a task done.
+ * Layer 1: Reject if any children are still non-terminal.
+ * Layer 2: If completionRule === "and", all terminal children must be "done".
+ * Layer 3: If metadata.repo is set, require a non-zeroed stateRef.
+ */
+function verifyCompletion(
+  core: Core,
+  task: Task,
+  stateRef?: StateRef,
+): RouteResult | null {
+  // (a) Task has metadata.repo but stateRef is missing or zeroed
+  if (task.metadata["repo"]) {
+    const ref = stateRef ?? task.stateRef;
+    if (!ref || isZeroedStateRef(ref)) {
+      return {
+        status: 422,
+        body: {
+          error: "missing_state_ref",
+          message:
+            "Task has metadata.repo but no valid stateRef was provided. " +
+            "Submit a stateRef with a real commit before marking done.",
+        },
+      };
+    }
+  }
+
+  // (b) Layer 1: Block if any children are still non-terminal
+  if (task.children.length > 0) {
+    const children = core.getChildren(task.id);
+    const pending = children.filter((c) => c.terminal === null);
+    if (pending.length > 0) {
+      return {
+        status: 422,
+        body: {
+          error: "children_pending",
+          message:
+            `Cannot mark done: ${pending.length} child(ren) still pending: ` +
+            pending.map((c) => c.id).join(", ") +
+            ". Cancel or complete them first.",
+        },
+      };
+    }
+
+    // (c) Layer 2: completionRule="and" requires all terminal children to be "done"
+    if (task.completionRule === "and") {
+      const notDone = children.filter((c) => c.terminal !== "done");
+      if (notDone.length > 0) {
+        return {
+          status: 422,
+          body: {
+            error: "children_not_done",
+            message:
+              `Task has completionRule='and' but ${notDone.length} child(ren) are not done: ` +
+              notDone.map((c) => c.id).join(", "),
+          },
+        };
+      }
+    }
+  }
+
+  return null;
+}
+
 function applyDoneTransition(
   core: Core,
   config: Config,
@@ -1732,6 +1800,10 @@ function applyDoneTransition(
   evidence?: string,
   stateRef?: StateRef,
 ): RouteResult {
+  // Pre-flight completion checks (children guard, stateRef guard)
+  const completionErr = verifyCompletion(core, task, stateRef);
+  if (completionErr) return completionErr;
+
   if (task.phase === "execution" && task.condition === "active" && task.reviewConfig === null) {
     // Verify artifacts before allowing completion
     const verification = verifyArtifacts(task, config);
@@ -2535,10 +2607,17 @@ function handleDecomposeCommit(
         }
       }
 
-      const parentPriority = (task.metadata["priority"] as string | undefined) ?? "medium";
-      const metadata: Record<string, unknown> = {};
+      // Inherit all parent metadata, then override with child-specific values
+      const metadata: Record<string, unknown> = { ...task.metadata };
+      // Remove parent-specific bookkeeping that shouldn't propagate
+      delete metadata["createdBy"]; delete metadata["createdAt"];
+      delete metadata["claimedAt"]; delete metadata["claimedBy"];
+      delete metadata["claimSessionId"]; delete metadata["claimSource"];
+      delete metadata["last_update"]; delete metadata["last_update_at"];
+      // Apply child overrides
       if (child.assignee) metadata["assignee"] = child.assignee;
       if (child.reviewer) metadata["reviewer"] = child.reviewer;
+      const parentPriority = (task.metadata["priority"] as string | undefined) ?? "medium";
       const childPriority = ("priority" in child ? (child as { priority?: string }).priority : undefined) ?? "medium";
       metadata["priority"] = maxPriority(parentPriority, childPriority);
 
@@ -2779,10 +2858,17 @@ function handleDecompose(
         }
       }
 
-      const parentPriority = (task.metadata["priority"] as string | undefined) ?? "medium";
-      const metadata: Record<string, unknown> = {};
+      // Inherit all parent metadata, then override with child-specific values
+      const metadata: Record<string, unknown> = { ...task.metadata };
+      // Remove parent-specific bookkeeping that shouldn't propagate
+      delete metadata["createdBy"]; delete metadata["createdAt"];
+      delete metadata["claimedAt"]; delete metadata["claimedBy"];
+      delete metadata["claimSessionId"]; delete metadata["claimSource"];
+      delete metadata["last_update"]; delete metadata["last_update_at"];
+      // Apply child overrides
       if (child.assignee) metadata["assignee"] = child.assignee;
       if (child.reviewer) metadata["reviewer"] = child.reviewer;
+      const parentPriority = (task.metadata["priority"] as string | undefined) ?? "medium";
       const childPriority = ("priority" in child ? (child as { priority?: string }).priority : undefined) ?? "medium";
       metadata["priority"] = maxPriority(parentPriority, childPriority);