fix(F167): a scheduler wake row was not a valid approval origin, so timer-woken sessions could never hand off

packages/api/src/domains/approval-hub/ApprovalIngress.ts

@@ -2,6 +2,7 @@ import type { ApprovalEnvelope, ApprovalOriginRef, ApprovalProducerId, CatId, Ri
 import { approvalProducerMeta, validateApprovalEnvelope, validateApprovalOriginRef } from '@cat-cafe/shared';
 import type { SocketManager } from '../../infrastructure/websocket/index.js';
 import type { IMessageStore, StoredMessage } from '../cats/services/stores/ports/MessageStore.js';
+import { isSystemUserMessage } from '../cats/services/stores/visibility.js';
 import { approvalCardIdempotencyKey, buildApprovalCardBlock } from './buildApprovalCardBlock.js';
 import type { ApprovalPublicationStore } from './ports/ApprovalPublicationStore.js';
 
@@ -218,7 +219,45 @@ export class ApprovalIngress {
     const origin = await this.deps.messageStore.getById(draft.originRef.messageId);
     if (!origin || origin.deletedAt || origin._tombstone) throw new Error('Approval origin message not found');
     if (origin.threadId !== draft.originRef.threadId) throw new Error('Approval origin message thread mismatch');
-    if (origin.userId !== draft.ownerUserId) throw new Error('Approval origin message owner mismatch');
+    // Cross-tenant isolation here is a CONJUNCTION of two parts, and neither part
+    // is sufficient on its own (@codex-luna's review, rounds 2-4):
+    //
+    //   (1) CALLER: originRef.threadId / .messageId and ownerUserId must be bound
+    //       to an authenticated record. This class CANNOT verify that binding.
+    //   (2) THIS CLASS: the assertion above checks that the origin message really
+    //       does live in the thread the draft names.
+    //
+    // (2) alone proves nothing about tenancy — a draft naming a thread of its own
+    // choosing satisfies the comparison trivially. It carries weight only because
+    // (1) fixes which thread may be named. On the F225 session-handoff path (1)
+    // holds: that producer derives all three from an authenticated
+    // InvocationRecord, and a request body cannot rewrite them.
+    //
+    // ApprovalIngress serves several producers, so (1) is a property of the F225
+    // path and NOT of this class. Any producer deriving a draft from untrusted
+    // input must itself establish and validate that binding before calling this
+    // ingress; the exemption below assumes (1) rather than proving it.
+    //
+    // Given (1) AND (2), what the comparison below still decides is narrower: who
+    // may have authored a row inside an already-verified thread — and a system
+    // pseudo-user speaking in that thread is not another tenant.
+    //
+    // Without the exemption this rejected precisely the sessions that need it most.
+    // A session-handoff proposal anchors on the message that triggered the
+    // invocation, and for any long-running session that message IS the scheduler's
+    // wake row ("持球唤醒"), persisted as userId='scheduler' / catId=null. So a
+    // timer-woken cat could never hand off, while the same proposal from an
+    // A2A-triggered turn (authored by the real owner) went through — the failure
+    // was invisible except as a 500 with no actionable text.
+    //
+    // isSystemUserMessage is the store layer's existing predicate for exactly this
+    // distinction, and it is deliberately reused rather than re-derived here: it
+    // requires BOTH a system userId AND a system/null catId, so a cat-authored row
+    // wearing a system userId stays rejected. A second, private definition of "is
+    // this the system" is how the two drift apart.
+    if (origin.userId !== draft.ownerUserId && !isSystemUserMessage(origin)) {
+      throw new Error('Approval origin message owner mismatch');
+    }
   }
 
   private async findPersistedCard(

packages/api/test/approval-hub/approval-ingress.test.js

@@ -41,14 +41,15 @@ class FakePublicationStore {
   }
 }
 
-function appendOrigin(messageStore) {
+function appendOrigin(messageStore, authorOverrides = {}) {
   messageStore.append({
     userId: ownerUserId,
     catId: null,
     content: 'please propose this',
     mentions: [],
     timestamp: 1_721_111_110_000,
     threadId: originRef.threadId,
+    ...authorOverrides,
   });
   const stored = messageStore.getByThread(originRef.threadId, 1)[0];
   stored.id = originRef.messageId;
@@ -76,9 +77,9 @@ function makeDraft(overrides = {}) {
   };
 }
 
-function makeHarness() {
+function makeHarness(originAuthorOverrides = {}) {
   const messageStore = new MessageStore();
-  appendOrigin(messageStore);
+  appendOrigin(messageStore, originAuthorOverrides);
   const broadcasts = [];
   const userEvents = [];
   const socketManager = {
@@ -100,6 +101,48 @@ function findApprovalCard(messageStore) {
 }
 
 describe('ApprovalIngress', () => {
+  // F167 — a session-handoff proposal anchors on the message that triggered the
+  // invocation, and for a long-running session that message IS the scheduler's
+  // wake row ("持球唤醒"), persisted under the `scheduler` system pseudo-user with
+  // `catId: null`. A bare `origin.userId !== ownerUserId` check therefore rejected
+  // exactly the sessions most in need of a handoff: every attempt driven by a
+  // timer wake failed with "Approval origin message owner mismatch", while the
+  // same proposal from an A2A-triggered turn (userId = the real owner) succeeded.
+  //
+  // The cross-user protection does not live in this comparison — the preceding
+  // threadId assertion already pins the origin to the caller's own thread. What
+  // this one adds is a rule about WHO may author a row inside that thread, and a
+  // system pseudo-user speaking in your own thread is not another tenant.
+  it('accepts a scheduler-authored origin in the owner thread', async () => {
+    const harness = makeHarness({ userId: 'scheduler', catId: null });
+    const store = new FakePublicationStore();
+
+    const envelope = await harness.ingress.publish(makeDraft(), store);
+
+    assert.equal(store.publication.state, 'anchored');
+    assert.equal(envelope.approvalCardRef.threadId, 'source-thread');
+  });
+
+  // The exemption must not become a hole. A different HUMAN owner is a genuine
+  // cross-tenant anchor and stays rejected — without this, the fix above would be
+  // indistinguishable from deleting the check.
+  it('still rejects an origin authored by another human user', async () => {
+    const harness = makeHarness({ userId: 'user-2', catId: null });
+    const store = new FakePublicationStore();
+
+    await assert.rejects(() => harness.ingress.publish(makeDraft(), store), /Approval origin message owner mismatch/);
+  });
+
+  // isSystemUserMessage requires BOTH a system userId AND a system/null catId, so
+  // a cat-authored row wearing a system userId is not a system message. Pinning it
+  // here keeps the exemption tied to that predicate rather than to the userId alone.
+  it('rejects a system userId carried by a cat-authored message', async () => {
+    const harness = makeHarness({ userId: 'scheduler', catId: 'codex-sol' });
+    const store = new FakePublicationStore();
+
+    await assert.rejects(() => harness.ingress.publish(makeDraft(), store), /Approval origin message owner mismatch/);
+  });
+
   it('persists one card, commits its exact envelope, then broadcasts', async () => {
     const harness = makeHarness();
     const store = new FakePublicationStore();