From 7d48708980463a1b68a83298e8b585fc5ca2f3bc Mon Sep 17 00:00:00 2001 From: Ragdoll-Opus-5 Date: Wed, 12 Aug 2026 15:30:12 +0300 Subject: [PATCH 01/10] fix(F167): a scheduler wake row was not a valid approval origin, so timer-woken sessions could never hand off MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit propose_session_handoff anchors its approval card on the message that triggered the invocation: const originMessageId = record.originTriggerMessageId ?? record.a2aTriggerMessageId; For any long-running session that message IS the scheduler's wake row ("持球唤醒"), which is persisted as userId='scheduler' / catId=null. ApprovalIngress.validateOrigin then compared origin.userId against the proposal's ownerUserId and threw: 500 Approval origin message owner mismatch So the sessions most in need of a handoff -- the ones long enough to be driven by timers, carrying the heaviest context -- were exactly the ones that could not propose one. Observed live: three consecutive attempts from an opus5 author line, all three woken by the scheduler, all three 500. The same proposal from an A2A-triggered turn would have succeeded, because a cross-thread delivery is persisted under the real owner's userId. That asymmetry is what made this look like an intermittent server fault rather than a rule. The check was not protecting what it appeared to protect. Cross-tenant isolation is the assertion one line ABOVE: the origin must live in the caller's own thread, and originRef.threadId comes from the authenticated callback record. What the userId comparison adds is a narrower rule about who may have authored a row inside that already-verified thread -- and a system pseudo-user speaking in your own thread is not another tenant. Fix: exempt system-authored rows via isSystemUserMessage, the store layer's existing predicate for this exact distinction. Deliberately reused rather than re-derived: 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 two such definitions drift apart. Evidence, not inference. The live wake row was read back from the store before any code was changed: { id: 0001786521900305-..., userId: "scheduler", catId: null, threadId: thread_mslv8bbw8pbazsz2 } with the thread owned by default-user -- same thread (so the threadId assertion passed), different userId (so this one threw). Tests: three cases, not one. The new red case pins the scheduler origin; the other two exist so that the fix cannot degrade into deleting the check -- another HUMAN owner must still be rejected, and a system userId carried by a cat-authored message must still be rejected. Verified red before green: exactly one failure, and it failed on `Approval origin message owner mismatch` thrown from validateOrigin, not on anything incidental. approval-ingress.test.js 16/16 (1 failing before the fix) approval-hub/*.test.js 299/299 session-handoff-* 54/54 Co-Authored-By: Claude Opus 5 --- .../domains/approval-hub/ApprovalIngress.ts | 23 +++++++- .../approval-hub/approval-ingress.test.js | 55 ++++++++++++++++++- 2 files changed, 74 insertions(+), 4 deletions(-) diff --git a/packages/api/src/domains/approval-hub/ApprovalIngress.ts b/packages/api/src/domains/approval-hub/ApprovalIngress.ts index 597b661495..e3d8c4192d 100644 --- a/packages/api/src/domains/approval-hub/ApprovalIngress.ts +++ b/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,27 @@ 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 is the assertion ABOVE: the origin must live in the + // caller's own thread. What remains here is a narrower question — who may have + // authored a row inside that thread — and a system pseudo-user speaking in your + // own 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( diff --git a/packages/api/test/approval-hub/approval-ingress.test.js b/packages/api/test/approval-hub/approval-ingress.test.js index bb1c4f97a6..9dd8ba14fe 100644 --- a/packages/api/test/approval-hub/approval-ingress.test.js +++ b/packages/api/test/approval-hub/approval-ingress.test.js @@ -41,7 +41,7 @@ class FakePublicationStore { } } -function appendOrigin(messageStore) { +function appendOrigin(messageStore, authorOverrides = {}) { messageStore.append({ userId: ownerUserId, catId: null, @@ -49,6 +49,7 @@ function appendOrigin(messageStore) { 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,54 @@ 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(); From 7843fb6fa978b126b2a454a926c748f0932240b9 Mon Sep 17 00:00:00 2001 From: Ragdoll-Opus-5 Date: Wed, 12 Aug 2026 15:59:08 +0300 Subject: [PATCH 02/10] docs(F167): record that the threadId equality is a caller invariant, not a check this class performs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 砚砚's review approved the fix and added one boundary worth more than a chat message: the preceding threadId assertion is a CALLER invariant, not an independent authorization check ApprovalIngress runs against an arbitrary draft. My comment said "cross-tenant isolation is the assertion ABOVE", which reads as though this class guarantees it. It does not. The guarantee holds because the only producer of this draft builds it from an authenticated InvocationRecord, so a request body cannot rewrite threadId, userId or the trigger messageId. Left in the review thread, that distinction decays; the next person to add a producer here would read my comment and assume protection that this class never provided. Written next to the exemption, it states what the exemption assumes. Comment only — no behaviour change. Re-verified: approval-hub + session-handoff 353/353. Co-Authored-By: Claude Opus 5 --- packages/api/src/domains/approval-hub/ApprovalIngress.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/api/src/domains/approval-hub/ApprovalIngress.ts b/packages/api/src/domains/approval-hub/ApprovalIngress.ts index e3d8c4192d..c027ea6a0d 100644 --- a/packages/api/src/domains/approval-hub/ApprovalIngress.ts +++ b/packages/api/src/domains/approval-hub/ApprovalIngress.ts @@ -224,6 +224,14 @@ export class ApprovalIngress { // authored a row inside that thread — and a system pseudo-user speaking in your // own thread is not another tenant. // + // Read that precisely (砚砚 review): the threadId equality is a CALLER + // invariant, not an authorization check this class performs on an arbitrary + // draft. It holds today because the only producer of this draft builds it from + // an authenticated InvocationRecord — request bodies cannot rewrite threadId, + // userId or the trigger messageId. A future producer that derives a draft from + // untrusted input must re-establish that binding itself; the exemption below + // assumes it rather than proving it. + // // 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 From 2bad996bcf5c0a298f3434f6acce8a0627d08f27 Mon Sep 17 00:00:00 2001 From: Ragdoll-Opus-5 Date: Wed, 12 Aug 2026 16:02:51 +0300 Subject: [PATCH 03/10] docs(F167): fix a misattributed review credit, and stop implying this class has one producer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two corrections to the comment added in 7843fb6fa, one of them mine to own. 1. I credited the review to 砚砚. The reviewer was @codex-luna. While reading sessionHandoffPropose.ts I absorbed its existing '(砚砚 P2)' annotations and reproduced that name without checking who had actually reviewed THIS change. Attribution is not decoration — a wrong name sends the next reader to the wrong cat for the reasoning behind the exemption. 2. @codex-luna's non-blocking P3, which is more than wording. I wrote 'the only producer of this draft'. Read as a statement about the class it is false: ApprovalIngress serves many producers (F128/F139/F193/F221/F225/F231/F246/ F260/F276). Worse, it is false in the dangerous direction — it invites the assumption that every path into this ingress carries an authenticated binding. Now scoped explicitly to the F225 handoff path, with the class-level caveat stated rather than implied, and 're-establish' replaced by his more precise 'establish and validate an authenticated owner/thread/origin binding before calling this ingress'. Worth naming the shape: the comment existed to prevent one misreading and introduced another one level up. Same failure mode, one layer removed — which is exactly what this branch keeps finding elsewhere. Comment only — no behaviour change. Re-verified: approval-hub + session-handoff 353/353, git diff --check clean. Co-Authored-By: Claude Opus 5 --- .../domains/approval-hub/ApprovalIngress.ts | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/packages/api/src/domains/approval-hub/ApprovalIngress.ts b/packages/api/src/domains/approval-hub/ApprovalIngress.ts index c027ea6a0d..676926feee 100644 --- a/packages/api/src/domains/approval-hub/ApprovalIngress.ts +++ b/packages/api/src/domains/approval-hub/ApprovalIngress.ts @@ -224,13 +224,17 @@ export class ApprovalIngress { // authored a row inside that thread — and a system pseudo-user speaking in your // own thread is not another tenant. // - // Read that precisely (砚砚 review): the threadId equality is a CALLER - // invariant, not an authorization check this class performs on an arbitrary - // draft. It holds today because the only producer of this draft builds it from - // an authenticated InvocationRecord — request bodies cannot rewrite threadId, - // userId or the trigger messageId. A future producer that derives a draft from - // untrusted input must re-establish that binding itself; the exemption below - // assumes it rather than proving it. + // Read that precisely (@codex-luna's review): the threadId equality is a + // CALLER invariant, not an authorization check this class performs on an + // arbitrary draft. It holds on the F225 session-handoff path because that + // producer builds the draft from an authenticated InvocationRecord — a request + // body cannot rewrite threadId, userId or the trigger messageId. + // + // ApprovalIngress serves several producers, so that 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 an authenticated + // owner/thread/origin binding before calling this ingress; the exemption below + // assumes that binding rather than proving it. // // Without the exemption this rejected precisely the sessions that need it most. // A session-handoff proposal anchors on the message that triggered the From bd609336d53b1de4c7f30b9d966f78b78ea3b4f5 Mon Sep 17 00:00:00 2001 From: Ragdoll-Opus-5 Date: Wed, 12 Aug 2026 16:06:49 +0300 Subject: [PATCH 04/10] =?UTF-8?q?docs(F167):=20name=20the=20caller=20invar?= =?UTF-8?q?iant=20correctly=20=E2=80=94=20it=20is=20the=20origin=20binding?= =?UTF-8?q?,=20not=20the=20threadId=20comparison?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @codex-luna's third pass caught that my previous wording said the opposite of the truth about this class. I wrote 'the threadId equality is a CALLER invariant, not an authorization check this class performs'. But this class DOES perform that comparison, one line above. What it cannot verify is whether the originRef it was handed — its threadId, and the ownerUserId compared below — came from an authenticated record at all. That binding is the caller invariant; the comparison is ours. So the sentence attributed our own check to the caller, and left the thing the caller actually owns unnamed. A reader auditing this exemption would have looked for the wrong guarantee in the wrong place. A comment that states the inverse of what the code does is worse than no comment: it survives review by sounding careful. Third correction to the same comment, each one from a reviewer pass, each moving it closer to what the code actually does. Worth noting that all three were about the SAME sentence — the prose kept drifting toward whatever sounded reassuring rather than what was checked where. Comment only — no behaviour change. Re-verified: approval-hub + session-handoff 353/353, git diff --check clean. Co-Authored-By: Claude Opus 5 --- .../domains/approval-hub/ApprovalIngress.ts | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/packages/api/src/domains/approval-hub/ApprovalIngress.ts b/packages/api/src/domains/approval-hub/ApprovalIngress.ts index 676926feee..faa15aaca3 100644 --- a/packages/api/src/domains/approval-hub/ApprovalIngress.ts +++ b/packages/api/src/domains/approval-hub/ApprovalIngress.ts @@ -219,16 +219,18 @@ 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'); - // Cross-tenant isolation is the assertion ABOVE: the origin must live in the - // caller's own thread. What remains here is a narrower question — who may have - // authored a row inside that thread — and a system pseudo-user speaking in your - // own thread is not another tenant. + // Cross-tenant isolation rests on the assertion ABOVE: the origin must live in + // the thread the draft names. What remains here is a narrower question — who + // may have authored a row inside that thread — and a system pseudo-user + // speaking in your own thread is not another tenant. // - // Read that precisely (@codex-luna's review): the threadId equality is a - // CALLER invariant, not an authorization check this class performs on an - // arbitrary draft. It holds on the F225 session-handoff path because that - // producer builds the draft from an authenticated InvocationRecord — a request - // body cannot rewrite threadId, userId or the trigger messageId. + // State that precisely (@codex-luna's review): the CALLER invariant is the + // AUTHENTICATED ORIGIN BINDING, not the threadId comparison. This class does + // perform that comparison; what it cannot verify is whether the originRef it + // was handed — threadId, and the ownerUserId compared below — came from an + // authenticated record at all. On the F225 session-handoff path they do: that + // producer derives them from an authenticated InvocationRecord, and a request + // body cannot rewrite them. // // ApprovalIngress serves several producers, so that is a property of the F225 // path and NOT of this class. Any producer deriving a draft from untrusted From e5b6b1c964c15e7d6f51da169b2004cef0185ac0 Mon Sep 17 00:00:00 2001 From: Ragdoll-Opus-5 Date: Wed, 12 Aug 2026 16:12:34 +0300 Subject: [PATCH 05/10] =?UTF-8?q?docs(F167):=20isolation=20is=20a=20conjun?= =?UTF-8?q?ction=20=E2=80=94=20stop=20crediting=20it=20to=20either=20half?= =?UTF-8?q?=20alone?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @codex-luna's fourth pass caught that the opening sentence still carried the exact error the last three commits fixed further down: "Cross-tenant isolation rests on the assertion ABOVE" credits tenancy to the threadId comparison. It does not hold alone. That comparison only proves the origin message lives in the thread the draft NAMES — a draft naming a thread of its own choosing satisfies it trivially. It carries weight only because the caller has already bound originRef.threadId/.messageId and ownerUserId to an authenticated record, which fixes WHICH thread may be named. Neither half is sufficient; isolation is the conjunction. The reason this took four rounds is the shape of the fix, not the difficulty of the claim. The comment stated the same security argument three times in prose, so each round I corrected the instance the reviewer quoted and left its paraphrases standing one paragraph up. Patching instances of a class. So this states the argument ONCE, as numbered parts (1) CALLER / (2) THIS CLASS, with what each cannot do written next to it, and deletes the restatements. There is no longer a second place for the claim to drift, which is the only version of this fix that ends the series rather than extending it. Comment only — no behaviour change. Re-verified: approval-hub + session-handoff 353/353 pass 0 fail, comment-only diff asserted mechanically (every changed line matches ^[+-]\s*//), git diff --check clean. Co-Authored-By: Claude Opus 5 --- .../domains/approval-hub/ApprovalIngress.ts | 34 +++++++++++-------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/packages/api/src/domains/approval-hub/ApprovalIngress.ts b/packages/api/src/domains/approval-hub/ApprovalIngress.ts index faa15aaca3..92893aeb07 100644 --- a/packages/api/src/domains/approval-hub/ApprovalIngress.ts +++ b/packages/api/src/domains/approval-hub/ApprovalIngress.ts @@ -219,24 +219,28 @@ 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'); - // Cross-tenant isolation rests on the assertion ABOVE: the origin must live in - // the thread the draft names. What remains here is a narrower question — who - // may have authored a row inside that thread — and a system pseudo-user - // speaking in your own thread is not another tenant. + // 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): // - // State that precisely (@codex-luna's review): the CALLER invariant is the - // AUTHENTICATED ORIGIN BINDING, not the threadId comparison. This class does - // perform that comparison; what it cannot verify is whether the originRef it - // was handed — threadId, and the ownerUserId compared below — came from an - // authenticated record at all. On the F225 session-handoff path they do: that - // producer derives them from an authenticated InvocationRecord, and a request - // body cannot rewrite them. + // (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. // - // ApprovalIngress serves several producers, so that is a property of the F225 + // (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 an authenticated - // owner/thread/origin binding before calling this ingress; the exemption below - // assumes that binding rather than proving it. + // 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 From c5d1214278dff8588133d716642c6278210475d3 Mon Sep 17 00:00:00 2001 From: Ragdoll-Opus-5 Date: Wed, 12 Aug 2026 16:50:54 +0300 Subject: [PATCH 06/10] style(F167): satisfy biome formatting in the two guard tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI Lint failed on `pnpm check`: biome wanted the two `assert.rejects` calls in the guard tests collapsed onto one line. Formatting only — no assertion, no matcher, and no behaviour changed. Worth recording why CI caught this and I did not: my local gate was `pnpm build` + the three test suites. The repo's own `pnpm check` — which is what the Lint job runs — was never in my loop. Passing the tests I chose is not the same as passing the checks the repo requires, and only the second one is the actual contract. Same shape as the rest of this branch: I verified the thing I was thinking about rather than the thing that gates the merge. Confined to the two tests this branch added; no unrelated files reformatted. Re-verified: pnpm check exit 0, approval-hub + session-handoff 353 pass / 0 fail. Co-Authored-By: Claude Opus 5 --- .../api/test/approval-hub/approval-ingress.test.js | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/packages/api/test/approval-hub/approval-ingress.test.js b/packages/api/test/approval-hub/approval-ingress.test.js index 9dd8ba14fe..1a86460946 100644 --- a/packages/api/test/approval-hub/approval-ingress.test.js +++ b/packages/api/test/approval-hub/approval-ingress.test.js @@ -130,10 +130,7 @@ describe('ApprovalIngress', () => { 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/, - ); + 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 @@ -143,10 +140,7 @@ describe('ApprovalIngress', () => { 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/, - ); + await assert.rejects(() => harness.ingress.publish(makeDraft(), store), /Approval origin message owner mismatch/); }); it('persists one card, commits its exact envelope, then broadcasts', async () => { From 82086f3ec64e312aa7478baec3cb465288d12758 Mon Sep 17 00:00:00 2001 From: Ragdoll-Opus-5 Date: Wed, 12 Aug 2026 17:14:34 +0300 Subject: [PATCH 07/10] fix(F167): scope the system-origin exemption to producers that declare the binding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Maintainer gate 2 on PR #1347: the exemption relaxed validateOrigin for EVERY message-origin producer, while the security argument only established the authenticated origin binding for the F225 session-handoff caller. The exemption was as wide as the shared ingress; the justification covered one path. So the binding is now DECLARED per producer and ENFORCED, not documented: `systemOriginExemption: 'server_attested' | 'forbidden'` on the producer catalog entry, consulted in validateOrigin. Only F225 is attested — its adapter reads originTriggerMessageId/a2aTriggerMessageId, threadId and userId off the authenticated InvocationRecord, and no request body can rewrite them. The field is REQUIRED, so a new producer cannot inherit the exemption by omission — leaving it out is a compile error, not a silent default. My own test was the evidence and I misread it. makeDraft() defaults to F128, so `accepts a scheduler-authored origin` was exercising the over-broad exemption on a producer that never proved the binding — and its green read as confirmation. That test now names F225 explicitly, and a new case asserts F128 with the same scheduler origin is still REJECTED. Proven load-bearing by mutation: forcing the guard true fails exactly that one case (16/17), so the scoping and no scoping are distinguishable. Also gate 3: the unit-test comment still claimed the threadId assertion alone pins the origin to the caller's own thread. That is the original wrong sentence, corrected four times in ApprovalIngress.ts across four review rounds and left standing here the whole time — every round fixed the instance being quoted and nobody grepped for the paraphrase. Worse, the falsifiability criterion I handed the reviewer was scoped "if this claim still appears in the FILE", which excluded the one place it did. A test written to pass. Verified: pnpm check exit 0 (the repo gate, not just my own), build clean, approval-hub + session-handoff 354 pass / 0 fail, mutation experiment as above. Refs #1348 Co-Authored-By: Claude Opus 5 --- .../domains/approval-hub/ApprovalIngress.ts | 18 ++++++--- .../approval-hub/approval-ingress.test.js | 40 ++++++++++++++++--- .../shared/src/approval-producer-catalog.ts | 28 +++++++++++++ 3 files changed, 75 insertions(+), 11 deletions(-) diff --git a/packages/api/src/domains/approval-hub/ApprovalIngress.ts b/packages/api/src/domains/approval-hub/ApprovalIngress.ts index 92893aeb07..485ba9ee95 100644 --- a/packages/api/src/domains/approval-hub/ApprovalIngress.ts +++ b/packages/api/src/domains/approval-hub/ApprovalIngress.ts @@ -233,10 +233,17 @@ export class ApprovalIngress { // 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. + // ApprovalIngress serves several producers, so (1) is a property of a PATH and + // never of this class. An earlier revision only documented that and let the + // exemption apply to every message-origin producer — which made the exemption + // as wide as the shared ingress while the argument for it covered one caller + // (maintainer review, PR #1347 gate 2). + // + // So (1) is now DECLARED per producer and ENFORCED here, not assumed: + // `systemOriginExemption: 'server_attested'` in the producer catalog is a + // claim that this producer's adapter binds originRef + ownerUserId to an + // authenticated InvocationRecord. It is a required field, so a new producer + // cannot inherit the exemption by omission — it must say so and be reviewed. // // 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 @@ -255,7 +262,8 @@ export class ApprovalIngress { // 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)) { + const systemOriginExempt = approvalProducerMeta(draft.producerId).systemOriginExemption === 'server_attested'; + if (origin.userId !== draft.ownerUserId && !(systemOriginExempt && isSystemUserMessage(origin))) { throw new Error('Approval origin message owner mismatch'); } } diff --git a/packages/api/test/approval-hub/approval-ingress.test.js b/packages/api/test/approval-hub/approval-ingress.test.js index 1a86460946..6062dc1c9d 100644 --- a/packages/api/test/approval-hub/approval-ingress.test.js +++ b/packages/api/test/approval-hub/approval-ingress.test.js @@ -109,20 +109,48 @@ describe('ApprovalIngress', () => { // 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 () => { + // The threadId assertion does NOT by itself pin the origin to the caller's own + // thread: it only proves the stored message belongs to the thread the draft + // NAMES. Tenancy holds as a conjunction — the producer binds originRef and + // ownerUserId to an authenticated InvocationRecord (fixing which thread may be + // named), and this ingress then checks the stored origin is consistent with it. + // + // Given both, what the userId comparison still decides is narrower: WHO may + // author a row inside an already-bound thread — and a system pseudo-user + // speaking in that thread is not another tenant. + // + // This comment previously stated the opposite. It survived four review rounds + // that corrected the same claim in ApprovalIngress.ts, because every round + // fixed the instance being quoted and nobody grepped for the paraphrase living + // here (maintainer review, PR #1347). + it('accepts a scheduler-authored origin for a server-attested producer', async () => { const harness = makeHarness({ userId: 'scheduler', catId: null }); const store = new FakePublicationStore(); - const envelope = await harness.ingress.publish(makeDraft(), store); + const envelope = await harness.ingress.publish(makeDraft({ producerId: 'F225' }), store); assert.equal(store.publication.state, 'anchored'); assert.equal(envelope.approvalCardRef.threadId, 'source-thread'); }); + // The exemption is scoped to producers that DECLARE the authenticated origin + // binding, because ApprovalIngress is shared and the argument only ever covered + // one caller. Without this case the scoping and no scoping are indistinguishable. + // + // Note this case used to pass as an ACCEPT: makeDraft() defaults to F128, so the + // first version of this fix exempted a producer that never proved the binding — + // and the passing test read as confirmation. Caught in maintainer review of + // PR #1347, not by me or by four local review rounds. + it('rejects a scheduler-authored origin for a producer without the attestation', async () => { + const harness = makeHarness({ userId: 'scheduler', catId: null }); + const store = new FakePublicationStore(); + + await assert.rejects( + () => harness.ingress.publish(makeDraft({ producerId: 'F128' }), store), + /Approval origin message owner mismatch/, + ); + }); + // 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. diff --git a/packages/shared/src/approval-producer-catalog.ts b/packages/shared/src/approval-producer-catalog.ts index 09f6756351..058cbe4659 100644 --- a/packages/shared/src/approval-producer-catalog.ts +++ b/packages/shared/src/approval-producer-catalog.ts @@ -16,6 +16,22 @@ export interface ApprovalProducerCatalogEntry { /** Feature-owned approve/reject endpoint base. */ decisionEndpointBase: string; sourcePolicy: 'message-required' | 'message-or-event'; + /** + * Whether this producer may use the system-authored origin exemption in + * `ApprovalIngress.validateOrigin`. + * + * `server_attested` asserts the producer derives `originRef.threadId`, + * `originRef.messageId` and `ownerUserId` from an authenticated + * InvocationRecord that a request body cannot rewrite. Only then is a system + * pseudo-user row inside the already-bound owner thread safe to accept as an + * origin, because the caller — not the request — fixed which thread and owner + * may be named. + * + * `forbidden` is the default for every producer that has not demonstrated that + * binding. The field is REQUIRED so that adding a producer without deciding + * this is a compile error rather than a silent inherited exemption. + */ + systemOriginExemption: 'server_attested' | 'forbidden'; history: boolean; /** Null keeps the producer on its existing binary reject path. */ humanDispositionReasonCodes: readonly HumanDispositionReasonCode[] | null; @@ -35,6 +51,7 @@ export const APPROVAL_PRODUCER_CATALOG = { colorToken: 'var(--semantic-info)', decisionEndpointBase: '/api/proposals', sourcePolicy: 'message-required', + systemOriginExemption: 'forbidden', history: true, humanDispositionReasonCodes: null, }, @@ -44,6 +61,7 @@ export const APPROVAL_PRODUCER_CATALOG = { colorToken: 'var(--semantic-warning, #f59e0b)', decisionEndpointBase: '/api/schedule-proposals', sourcePolicy: 'message-or-event', + systemOriginExemption: 'forbidden', history: true, humanDispositionReasonCodes: null, }, @@ -53,6 +71,7 @@ export const APPROVAL_PRODUCER_CATALOG = { colorToken: 'var(--semantic-success, #22c55e)', decisionEndpointBase: '/api/dispatch-proposals', sourcePolicy: 'message-or-event', + systemOriginExemption: 'forbidden', history: true, humanDispositionReasonCodes: null, }, @@ -62,6 +81,7 @@ export const APPROVAL_PRODUCER_CATALOG = { colorToken: 'var(--accent-taste, #e879f9)', decisionEndpointBase: '/api/taste-proposals', sourcePolicy: 'message-or-event', + systemOriginExemption: 'forbidden', history: true, humanDispositionReasonCodes: null, }, @@ -71,6 +91,10 @@ export const APPROVAL_PRODUCER_CATALOG = { colorToken: 'var(--semantic-secondary, #8b5cf6)', decisionEndpointBase: '/api/session-handoff', sourcePolicy: 'message-required', + // callback-propose-session-handoff reads originTriggerMessageId / + // a2aTriggerMessageId, threadId and userId off the authenticated + // InvocationRecord; the request body cannot rewrite any of them. + systemOriginExemption: 'server_attested', history: true, humanDispositionReasonCodes: HUMAN_DISPOSITION_REASON_CODES, }, @@ -80,6 +104,7 @@ export const APPROVAL_PRODUCER_CATALOG = { colorToken: 'var(--semantic-warning, #f59e0b)', decisionEndpointBase: '/api/profile-updates', sourcePolicy: 'message-required', + systemOriginExemption: 'forbidden', history: true, humanDispositionReasonCodes: null, }, @@ -89,6 +114,7 @@ export const APPROVAL_PRODUCER_CATALOG = { colorToken: 'var(--accent-entity, #06b6d4)', decisionEndpointBase: '/api/entity-proposals', sourcePolicy: 'message-or-event', + systemOriginExemption: 'forbidden', history: true, humanDispositionReasonCodes: null, }, @@ -98,6 +124,7 @@ export const APPROVAL_PRODUCER_CATALOG = { colorToken: 'var(--accent-people, #14b8a6)', decisionEndpointBase: '/api/person-memory-proposals', sourcePolicy: 'message-required', + systemOriginExemption: 'forbidden', history: true, humanDispositionReasonCodes: ['not_important', 'wrong_lane', 'bad_evidence', 'wrong', 'other'], }, @@ -107,6 +134,7 @@ export const APPROVAL_PRODUCER_CATALOG = { colorToken: 'var(--semantic-info, #3b82f6)', decisionEndpointBase: '/api/meeting-intakes', sourcePolicy: 'message-or-event', + systemOriginExemption: 'forbidden', history: true, humanDispositionReasonCodes: null, }, From 717d9e5b4cda2196ccd5b9384431a67a76ab7dee Mon Sep 17 00:00:00 2001 From: Ragdoll-Opus-5 Date: Thu, 13 Aug 2026 00:17:10 +0300 Subject: [PATCH 08/10] =?UTF-8?q?fix(F167):=20four=20producers=20do=20bind?= =?UTF-8?q?=20=E2=80=94=20attesting=20them=20instead=20of=20re-seeding=20t?= =?UTF-8?q?he=20500?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @codex-luna blocked 82086f3ec: F128/F193/F231/F260 were marked `forbidden`, but their real callback adapters bind owner/thread/origin off the authenticated InvocationRecord exactly as F225 does. Leaving them forbidden would reintroduce the owner-mismatch 500 for scheduler-driven proposals — the precise failure this branch exists to remove. He is right; they are now `server_attested`. I verified rather than took the audit on faith, and the result is one notch looser than stated. Two binding strengths, now recorded per entry instead of flattened into one word: DIRECT F225, and F231 — a body-supplied sourceMessageId is rejected unless it equals the record-derived originMessageId, else omitted. TRANSITIVE F128 — callback-propose-thread writes the proposal row wholly from the record (sourceThreadId=record.threadId, sourceMessageId=record origin trigger, createdBy=record.userId) and builds originRef from that row. Request body cannot rewrite any of the three. Walked at the creation site by me. F193/F260 same shape; their creation sites are @codex-luna's audit, attributed as such rather than presented as mine. The four remaining `forbidden` values now carry their reason (F139 event-origin early return, F221 genuinely unbound, F276 deferred-receipt sourceRef, F292 never reaches this ingress), so a reader can tell deliberate from unfilled. My own negative case was the other half of the mistake. It used a fabricated F128 draft — a producer that DOES bind — so it asserted a regression of the 500 and its green read as confirmation. Subject changed to F221, which genuinely has no binding, and a new case asserts F128 must be ACCEPTED so flipping it back fails loudly instead of silently restoring the bug. P2: dropped "DECLARED and ENFORCED here". The catalog is a capability gate over a declaration; required stops an omission, not a wrong declaration, and this class cannot verify the adapter. It narrows blast radius from every message-origin producer to the ones someone audited and signed for — a real reduction, not a verification. Proven in both directions this time, because one direction is what let the last round pass: forcing the exemption global kills only the F221 negative (17/1); flipping F128 back to forbidden kills only the new F128 accept guard (17/1). Also: pnpm check exit 0, build clean, 355 pass / 0 fail. Refs #1348 Co-Authored-By: Claude Opus 5 --- .../domains/approval-hub/ApprovalIngress.ts | 17 ++++--- .../approval-hub/approval-ingress.test.js | 31 ++++++++++--- .../shared/src/approval-producer-catalog.ts | 44 +++++++++++++++++-- 3 files changed, 78 insertions(+), 14 deletions(-) diff --git a/packages/api/src/domains/approval-hub/ApprovalIngress.ts b/packages/api/src/domains/approval-hub/ApprovalIngress.ts index 485ba9ee95..f7ce66ebce 100644 --- a/packages/api/src/domains/approval-hub/ApprovalIngress.ts +++ b/packages/api/src/domains/approval-hub/ApprovalIngress.ts @@ -239,11 +239,18 @@ export class ApprovalIngress { // as wide as the shared ingress while the argument for it covered one caller // (maintainer review, PR #1347 gate 2). // - // So (1) is now DECLARED per producer and ENFORCED here, not assumed: - // `systemOriginExemption: 'server_attested'` in the producer catalog is a - // claim that this producer's adapter binds originRef + ownerUserId to an - // authenticated InvocationRecord. It is a required field, so a new producer - // cannot inherit the exemption by omission — it must say so and be reviewed. + // So the exemption is no longer global: it is gated per producer by + // `systemOriginExemption` in the producer catalog, and a required field means a + // new producer cannot inherit it by omission. + // + // Be precise about what that gate is (@codex-luna, PR #1349 P2). It is a + // CAPABILITY GATE over a DECLARATION — the catalog says which producers claim + // the binding of (1). It is NOT a runtime proof that the named adapter really + // binds: this class cannot verify that, and a wrong `server_attested` value + // would not be caught here. The proof lives in the per-entry audit trail plus + // route-level coverage. So the gate narrows the blast radius from "every + // message-origin producer" to "the ones someone audited and signed for" — which + // is a real reduction, not a verification. // // 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 diff --git a/packages/api/test/approval-hub/approval-ingress.test.js b/packages/api/test/approval-hub/approval-ingress.test.js index 6062dc1c9d..f2d07a5875 100644 --- a/packages/api/test/approval-hub/approval-ingress.test.js +++ b/packages/api/test/approval-hub/approval-ingress.test.js @@ -137,20 +137,41 @@ describe('ApprovalIngress', () => { // binding, because ApprovalIngress is shared and the argument only ever covered // one caller. Without this case the scoping and no scoping are indistinguishable. // - // Note this case used to pass as an ACCEPT: makeDraft() defaults to F128, so the - // first version of this fix exempted a producer that never proved the binding — - // and the passing test read as confirmation. Caught in maintainer review of - // PR #1347, not by me or by four local review rounds. + // F221 is the honest negative: its `sourceMessageId` may be supplied by the + // request body and the derive path does not tie it back to the InvocationRecord, + // so it really has not established the binding. + // + // Two review layers landed on this one case. The maintainer (PR #1347) caught that + // the exemption was as wide as the shared ingress while the argument covered one + // caller. Then @codex-luna (PR #1349) caught that my first negative used F128 — + // a producer that DOES bind (record.threadId / record.originTriggerMessageId / + // record.userId, verified at the creation site) — so the case was fossilising a + // regression of the very 500 this branch exists to fix, and its green read as + // proof. A negative case is only evidence if its subject genuinely lacks the + // property; picking the wrong subject makes the assertion cosmetic. it('rejects a scheduler-authored origin for a producer without the attestation', async () => { const harness = makeHarness({ userId: 'scheduler', catId: null }); const store = new FakePublicationStore(); await assert.rejects( - () => harness.ingress.publish(makeDraft({ producerId: 'F128' }), store), + () => harness.ingress.publish(makeDraft({ producerId: 'F221' }), store), /Approval origin message owner mismatch/, ); }); + // Regression guard for the producer Luna's audit reclassified: F128 binds, so a + // scheduler-authored origin must be ACCEPTED for it. If someone flips F128 back to + // `forbidden`, this fails instead of silently restoring the owner-mismatch 500. + it('accepts a scheduler-authored origin for F128, whose binding is transitive but record-derived', async () => { + const harness = makeHarness({ userId: 'scheduler', catId: null }); + const store = new FakePublicationStore(); + + const envelope = await harness.ingress.publish(makeDraft({ producerId: 'F128' }), 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. diff --git a/packages/shared/src/approval-producer-catalog.ts b/packages/shared/src/approval-producer-catalog.ts index 058cbe4659..75017a13cf 100644 --- a/packages/shared/src/approval-producer-catalog.ts +++ b/packages/shared/src/approval-producer-catalog.ts @@ -30,6 +30,25 @@ export interface ApprovalProducerCatalogEntry { * `forbidden` is the default for every producer that has not demonstrated that * binding. The field is REQUIRED so that adding a producer without deciding * this is a compile error rather than a silent inherited exemption. + * + * WHAT THIS FIELD IS NOT (@codex-luna, PR #1349 P2): it is a policy DECLARATION, + * not a runtime proof that the named adapter actually binds. `required` stops an + * omission; it cannot stop a WRONG declaration. Proof has to come from route-level + * / integration coverage or a typed adapter binding — so treat a `server_attested` + * value as a claim that must have an audit trail next to it, which is why each + * entry below records who walked which creation site. + * + * Current `forbidden` values are deliberate, each for its own reason: + * F139 — publication always constructs an EVENT origin, and validateOrigin + * returns early for `kind === 'event'`, so the exemption is inert here. + * F221 — genuinely unbound: `sourceMessageId` may be supplied by the request + * body and the derive path does NOT tie it back to the InvocationRecord. + * This is the honest negative case for the ingress test. + * F276 — candidate `sourceRef` can come from an owner-validated deferred receipt + * or owner evidence rather than one InvocationRecord; needs scheduler / + * deferred route coverage before it could be attested. + * F292 — MeetingIntake lands through signal admission and never calls + * ApprovalIngress.publish, so this value has no effect on this ingress. */ systemOriginExemption: 'server_attested' | 'forbidden'; history: boolean; @@ -51,7 +70,12 @@ export const APPROVAL_PRODUCER_CATALOG = { colorToken: 'var(--semantic-info)', decisionEndpointBase: '/api/proposals', sourcePolicy: 'message-required', - systemOriginExemption: 'forbidden', + // TRANSITIVE binding. callback-propose-thread-routes writes the proposal row + // entirely off the authenticated record — sourceThreadId=record.threadId, + // sourceMessageId=record.originTriggerMessageId ?? record.a2aTriggerMessageId, + // createdBy=record.userId — and then builds originRef from that row. A request + // body cannot rewrite any of the three. Verified at the creation site by opus5. + systemOriginExemption: 'server_attested', history: true, humanDispositionReasonCodes: null, }, @@ -71,7 +95,11 @@ export const APPROVAL_PRODUCER_CATALOG = { colorToken: 'var(--semantic-success, #22c55e)', decisionEndpointBase: '/api/dispatch-proposals', sourcePolicy: 'message-or-event', - systemOriginExemption: 'forbidden', + // deriveCallbackOriginRef takes messageId off the authenticated record; threadId + // is either the authenticated actor's (DIRECT) or the persisted proposal's + // sourceThreadId (TRANSITIVE, same shape as F128). Creation-site audit by + // @codex-luna, PR #1349 review — not independently re-walked by opus5. + systemOriginExemption: 'server_attested', history: true, humanDispositionReasonCodes: null, }, @@ -104,7 +132,11 @@ export const APPROVAL_PRODUCER_CATALOG = { colorToken: 'var(--semantic-warning, #f59e0b)', decisionEndpointBase: '/api/profile-updates', sourcePolicy: 'message-required', - systemOriginExemption: 'forbidden', + // DIRECT binding, and the strictest of the set: a body-supplied sourceMessageId + // is rejected unless it equals the record-derived originMessageId + // (callback-propose-profile-update-routes.ts: `sourceMessageId !== originMessageId` + // → reject); omitting it falls back to the record value. Verified by opus5. + systemOriginExemption: 'server_attested', history: true, humanDispositionReasonCodes: null, }, @@ -114,7 +146,11 @@ export const APPROVAL_PRODUCER_CATALOG = { colorToken: 'var(--accent-entity, #06b6d4)', decisionEndpointBase: '/api/entity-proposals', sourcePolicy: 'message-or-event', - systemOriginExemption: 'forbidden', + // deriveEntityOriginRef is the same shape as F193: messageId off the + // authenticated record, threadId from the actor or the persisted proposal row. + // Creation-site audit by @codex-luna, PR #1349 review — not independently + // re-walked by opus5. + systemOriginExemption: 'server_attested', history: true, humanDispositionReasonCodes: null, }, From f21eeea27e9c658b9f3d7a925862301f6dc6c5af Mon Sep 17 00:00:00 2001 From: Ragdoll-Opus-5 Date: Thu, 13 Aug 2026 00:44:57 +0300 Subject: [PATCH 09/10] test(F167): prove the exemption's premise, instead of declaring it `systemOriginExemption: 'server_attested'` is the strongest claim on this branch and it was the one thing nothing executed. approval-ingress.test.js constructs its drafts at the ingress boundary, so all five cases prove validateOrigin DECIDES correctly given a producerId and an origin row. None of them reaches the premise that makes the decision safe: that the producer's real route derives threadId, messageId and ownerUserId from the authenticated InvocationRecord. `required` stops an omission. It cannot stop a WRONG declaration -- and a wrong one is invisible: flip a route to read sourceMessageId off the request body and every existing test stays green while the exemption turns back into the cross-tenant hole it was scoped to avoid. So these tests drive real HTTP routes with a real InvocationRegistry record whose origin trigger is a real scheduler wake row. Two files, deliberately: * approval-hub/scheduler-origin-callback-integration.test.js -- breadth. The same wake row across every attested producer (F225/F128/F193/F231/F260 accept) plus F221, which is `forbidden`, rejected on the identical path. That last case is what keeps the field a boundary rather than a blanket. * propose-session-handoff-route.test.js -- depth on F225, the reported defect. Adds the assertion neither the matrix nor the ingress suite makes: a body naming another thread / message / owner cannot move the persisted originRef. A status code cannot distinguish a route that validates those fields from one that ignores them; only the persisted origin can. Also covers the historical catId:'system' wake shape, and re-asserts both rejections (foreign human author, cat wearing a system userId) at route level rather than trusting that the route reaches the ingress at all. Each file cross-references the other, because two entry points that do not know about each other is how one of them gets deleted as redundant. Sensitivity measured, not assumed. Flipping F225 to `forbidden` turns exactly the three accept cases red and leaves both negatives green -- so the accepts do exercise the catalog gate, and the negatives do not depend on it being open. A green suite proves nothing about what it would catch. Closes @codex-luna's P2 (his only remaining finding at 717d9e5b4) and upgrades the maintainer's direction gate 2 from a tracked deferral to actual coverage. #1350 stays open for what is still not proven: F193/F260 rest on a creation-site audit I did not independently re-walk. 333/333 green across test/approval-hub/*, propose-session-handoff-route, session-handoff-propose, session-handoff-recovery. Note on authorship: the breadth file was written by a parallel invocation of this same cat sharing this worktree while I was writing the depth file -- we both took the same finding. Its work is committed verbatim. That collision also surfaced a hazard worth naming: packages/*/dist is shared mutable state across parallel invocations in one worktree, so a mutation build in one produces false reds in the other. I hit exactly that and nearly filed it as a defect in this file. Co-Authored-By: Claude Opus 5 --- ...eduler-origin-callback-integration.test.js | 369 ++++++++++++++++++ .../propose-session-handoff-route.test.js | 125 ++++++ 2 files changed, 494 insertions(+) create mode 100644 packages/api/test/approval-hub/scheduler-origin-callback-integration.test.js diff --git a/packages/api/test/approval-hub/scheduler-origin-callback-integration.test.js b/packages/api/test/approval-hub/scheduler-origin-callback-integration.test.js new file mode 100644 index 0000000000..7406d6cb5e --- /dev/null +++ b/packages/api/test/approval-hub/scheduler-origin-callback-integration.test.js @@ -0,0 +1,369 @@ +/** + * F167 — the system-origin exemption, exercised through real callback routes. + * + * Why this file exists (@codex-luna PR #1349 P2, and the clowder-ai maintainer on #1347): + * approval-ingress.test.js proves the exemption against FABRICATED drafts. A fabricated + * draft cannot show that a producer's real adapter ever puts the scheduler's wake row into + * originRef — and that gap already produced a wrong test once: the original negative case + * asserted rejection for F128, a producer that DOES bind, so it pinned a regression of the + * very 500 this branch removes, and its green read as confirmation. + * + * So nothing here is constructed at the ingress boundary. Each case drives the real HTTP + * route, authenticated by a real InvocationRegistry record, whose origin trigger is a real + * scheduler wake row appended the way infrastructure/scheduler/delivery.ts appends it. + * + * The five `server_attested` producers must publish. F221 — deliberately `forbidden` — must + * still be rejected on the identical path, which is what keeps the exemption a boundary + * rather than a blanket. + */ + +import '../helpers/setup-cat-registry.js'; +import assert from 'node:assert/strict'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { describe, it } from 'node:test'; +import Fastify from 'fastify'; +import { proposedReviewAction } from './helpers.js'; + +const OWNER = 'user-1'; +const CAT = 'opus'; + +const load = (path) => import(`../../dist/${path}.js`); +const socket = () => ({ emitToUser() {}, broadcastToRoom() {}, broadcastAgentMessage() {} }); + +/** + * The exact row infrastructure/scheduler/delivery.ts persists for a 持球唤醒 wake: + * authored by the `scheduler` pseudo-user with a null catId, never by the session owner. + */ +async function appendSchedulerWakeRow(messageStore, threadId) { + const row = await messageStore.append({ + userId: 'scheduler', + catId: null, + content: '[hold-ball] the condition you were waiting on expired', + mentions: [], + origin: 'callback', + timestamp: Date.now(), + threadId, + source: { connector: 'scheduler', label: '定时任务', icon: 'scheduler' }, + extra: { scheduler: { hiddenTrigger: true } }, + }); + // Fixture self-check. If this ever drifts back to an owner-authored row, every positive + // case below keeps passing with the exemption deleted — the file would assert nothing. + assert.equal(row.userId, 'scheduler', 'the wake row must be scheduler-authored'); + assert.equal(row.catId, null, 'a cat-authored row wearing a system userId must not be the fixture'); + assert.notEqual(row.userId, OWNER, 'owner-authored origins never needed the exemption'); + return row; +} + +/** + * invoke-single-cat.ts passes the turn's trigger id as arg 7 (originTriggerMessageId) and + * leaves args 4-6 undefined on the scheduler path. Arg 5 is a2aTriggerMessageId: putting the + * wake id there also compiles, yields an A2A-shaped record, and would test nothing. + * + * The record's userId is the OWNER. Only the message row is the scheduler's — that + * asymmetry between record and row is the entire bug. + */ +const schedulerWokenAuth = (registry, threadId, wakeMessageId) => + registry.create(OWNER, CAT, threadId, undefined, undefined, undefined, wakeMessageId); + +const post = (app, url, auth, payload) => + app.inject({ + method: 'POST', + url, + headers: { 'x-invocation-id': auth.invocationId, 'x-callback-token': auth.callbackToken }, + payload, + }); + +async function assertAnchoredToWakeRow(store, proposalId, threadId, wakeRow) { + const publication = await store.getPublication(proposalId); + assert.equal(publication?.state, 'anchored', 'the proposal must reach an anchored publication'); + assert.deepEqual( + publication.envelope.originRef, + { kind: 'message', threadId, messageId: wakeRow.id }, + 'the published origin must be the scheduler wake row itself, not a substitute', + ); +} + +describe('F167 scheduler-origin exemption over real callback routes', () => { + it('F225 — a timer-woken session can hand off (the exact failure this branch removes)', async () => { + const { InvocationRegistry } = await load('domains/cats/services/agents/invocation/InvocationRegistry'); + const { MessageStore } = await load('domains/cats/services/stores/ports/MessageStore'); + const { InMemorySessionHandoffProposalStore } = await load( + 'domains/cats/services/stores/ports/SessionHandoffProposalStore', + ); + const { callbacksRoutes } = await load('routes/index'); + + const registry = new InvocationRegistry(); + const messageStore = new MessageStore(); + const store = new InMemorySessionHandoffProposalStore(); + const session = { id: 'sess_active', status: 'active', catId: CAT, threadId: 'thread_1', userId: OWNER }; + + const app = Fastify(); + await app.register(callbacksRoutes, { + registry, + messageStore, + socketManager: socket(), + handoffProposalStore: store, + sessionChainStore: { + getActive: async (catId, threadId) => (catId === session.catId && threadId === session.threadId ? session : null), + }, + evidenceStore: { ingestRaw() {}, search: () => [] }, + markerQueue: { enqueue() {} }, + reflectionService: { reflect() {} }, + }); + await app.ready(); + + try { + const wake = await appendSchedulerWakeRow(messageStore, 'thread_1'); + const auth = await schedulerWokenAuth(registry, 'thread_1', wake.id); + + const res = await post(app, '/api/callbacks/propose-session-handoff', auth, { + done: 'ran long enough to be driven by the timer', + nextSteps: 'hand the rest to the next session', + }); + + assert.equal(res.statusCode, 200, res.body); + assert.equal(res.json().status, 'pending'); + await assertAnchoredToWakeRow(store, res.json().proposalId, 'thread_1', wake); + } finally { + await app.close(); + } + }); + + it('F128 — a scheduler-woken turn can propose a thread', async () => { + const { InvocationRegistry } = await load('domains/cats/services/agents/invocation/InvocationRegistry'); + const { MessageStore } = await load('domains/cats/services/stores/ports/MessageStore'); + const { ThreadStore } = await load('domains/cats/services/stores/ports/ThreadStore'); + const { InMemoryProposalStore } = await load('domains/cats/services/stores/ports/ProposalStore'); + const { callbacksRoutes } = await load('routes/index'); + + const registry = new InvocationRegistry(); + const messageStore = new MessageStore(); + const threadStore = new ThreadStore(); + const store = new InMemoryProposalStore(); + const source = await threadStore.create(OWNER, 'Source'); + + const app = Fastify(); + await app.register(callbacksRoutes, { + registry, + messageStore, + socketManager: socket(), + threadStore, + proposalStore: store, + evidenceStore: { ingestRaw() {}, search: () => [] }, + markerQueue: { enqueue() {} }, + reflectionService: { reflect() {} }, + }); + await app.ready(); + + try { + const wake = await appendSchedulerWakeRow(messageStore, source.id); + const auth = await schedulerWokenAuth(registry, source.id, wake.id); + + const res = await post(app, '/api/callbacks/propose-thread', auth, { + title: 'Split the timer-driven work out', + reason: 'The wake turn found a second track worth its own thread', + }); + + assert.equal(res.statusCode, 200, res.body); + await assertAnchoredToWakeRow(store, res.json().proposalId, source.id, wake); + } finally { + await app.close(); + } + }); + + it('F193 — a scheduler-woken turn can propose a cross-thread dispatch', async () => { + const { ApprovalIngress } = await load('domains/approval-hub/ApprovalIngress'); + const { InvocationRegistry } = await load('domains/cats/services/agents/invocation/InvocationRegistry'); + const { MessageStore } = await load('domains/cats/services/stores/ports/MessageStore'); + const { ThreadStore } = await load('domains/cats/services/stores/ports/ThreadStore'); + const { InMemoryDispatchProposalStore } = await load('domains/approval-hub/stores/ports/IDispatchProposalStore'); + const { callbacksRoutes } = await load('routes/callbacks'); + + const registry = new InvocationRegistry(); + const messageStore = new MessageStore(); + const threadStore = new ThreadStore(); + const store = new InMemoryDispatchProposalStore(); + const source = await threadStore.create(OWNER, 'Source'); + const target = await threadStore.create(OWNER, 'Target'); + await threadStore.addParticipants(source.id, [CAT]); + await threadStore.addParticipants(target.id, ['sonnet']); + + const app = Fastify(); + await app.register(callbacksRoutes, { + registry, + messageStore, + threadStore, + socketManager: socket(), + router: { async *routeExecution() {}, getExecutions: () => [] }, + invocationRecordStore: { create: () => ({ outcome: 'created' }), update() {}, get: () => null }, + dispatchProposalStore: store, + approvalIngress: new ApprovalIngress({ messageStore, socketManager: socket() }), + }); + await app.ready(); + + try { + const wake = await appendSchedulerWakeRow(messageStore, source.id); + const auth = await schedulerWokenAuth(registry, source.id, wake.id); + + const res = await post(app, '/api/callbacks/post-message', auth, { + threadId: target.id, + content: '@sonnet\nThe timer woke me and this needs another pair of paws', + targetCats: ['sonnet'], + effectClass: 'assign_work', + proposedAction: proposedReviewAction(), + clientMessageId: 'f167-scheduler-origin-dispatch', + }); + + assert.equal(res.statusCode, 200, res.body); + const [proposal] = await store.listPendingByUser(OWNER); + assert.ok(proposal, 'the dispatch intercept must have created a proposal'); + await assertAnchoredToWakeRow(store, proposal.proposalId, source.id, wake); + } finally { + await app.close(); + } + }); + + it('F231 — a scheduler-woken turn can propose a profile update', async () => { + const { InvocationRegistry } = await load('domains/cats/services/agents/invocation/InvocationRegistry'); + const { MessageStore } = await load('domains/cats/services/stores/ports/MessageStore'); + const { InMemoryProfileUpdateProposalStore } = await load( + 'domains/cats/services/stores/ports/ProfileUpdateProposalStore', + ); + const { FileProfileRepository } = await load('domains/cats/services/profile/ProfileRepository'); + const { registerCallbackAuthHook } = await load('routes/callback-auth-prehandler'); + const { registerCallbackProposeProfileUpdateRoutes } = await load('routes/callback-propose-profile-update-routes'); + + const registry = new InvocationRegistry(); + const messageStore = new MessageStore(); + const store = new InMemoryProfileUpdateProposalStore(); + const dataDir = mkdtempSync(join(tmpdir(), 'f167-scheduler-origin-')); + + const app = Fastify(); + registerCallbackAuthHook(app, registry); + registerCallbackProposeProfileUpdateRoutes(app, { + registry, + proposalStore: store, + messageStore, + socketManager: socket(), + repository: new FileProfileRepository({ dataDir, relationshipKeyForCat: () => 'ragdoll' }), + }); + await app.ready(); + + try { + const wake = await appendSchedulerWakeRow(messageStore, 'thread_1'); + const auth = await schedulerWokenAuth(registry, 'thread_1', wake.id); + + const res = await post(app, '/api/callbacks/propose-profile-update', auth, { + afterContent: 'NEW primer written during a timer-driven turn', + rationale: 'Observed while the scheduler had the ball', + signalKind: 'cat-declared', + }); + + assert.equal(res.statusCode, 200, res.body); + const [proposal] = await store.listPending(OWNER); + assert.ok(proposal, 'the profile-update route must have created a proposal'); + await assertAnchoredToWakeRow(store, proposal.proposalId, 'thread_1', wake); + } finally { + await app.close(); + rmSync(dataDir, { recursive: true, force: true }); + } + }); + + it('F260 — a scheduler-woken turn can propose an entity', async () => { + const { ApprovalIngress } = await load('domains/approval-hub/ApprovalIngress'); + const { InvocationRegistry } = await load('domains/cats/services/agents/invocation/InvocationRegistry'); + const { MessageStore } = await load('domains/cats/services/stores/ports/MessageStore'); + const { InMemoryEntityProposalStore } = await load('domains/approval-hub/stores/ports/IEntityProposalStore'); + const { callbackProposeEntityRoutes } = await load('routes/callback-propose-entity-routes'); + + const registry = new InvocationRegistry(); + const messageStore = new MessageStore(); + const store = new InMemoryEntityProposalStore(); + + const app = Fastify(); + await app.register(callbackProposeEntityRoutes, { + registry, + entityProposalStore: store, + messageStore, + socketManager: socket(), + approvalIngress: new ApprovalIngress({ messageStore, socketManager: socket() }), + }); + await app.ready(); + + try { + const wake = await appendSchedulerWakeRow(messageStore, 'thread_1'); + const auth = await schedulerWokenAuth(registry, 'thread_1', wake.id); + + const res = await post(app, '/api/callbacks/propose-entity', auth, { + entityId: 'concept:hold-ball-wake', + entityType: 'concept', + canonicalName: 'Hold-ball wake', + aliases: ['持球唤醒'], + stance: 'endorsed', + visibilityScope: 'workspace', + provenance: [{ source: 'f167-scheduler-origin-integration' }], + rationale: 'Named during a timer-driven turn', + clientRequestId: 'f167-scheduler-origin-entity', + }); + + assert.equal(res.statusCode, 200, res.body); + await assertAnchoredToWakeRow(store, res.json().proposalId, 'thread_1', wake); + } finally { + await app.close(); + } + }); + + it('F221 — a producer whose adapter permits a body-chosen origin gets no exemption on the same path', async () => { + // Not "this request's origin is unbound" — here it comes off the record exactly as F225's + // does. The catalog gate is per PRODUCER, not per request: F221's deriveTasteOriginRef + // prefers a body-supplied sourceMessageId with no equality check, so no request of its + // shape can be attested. This is the negative the earlier fabricated-F128 case should + // have been, and it is the case that dies if anyone widens the exemption back to global. + const { ApprovalIngress } = await load('domains/approval-hub/ApprovalIngress'); + const { InvocationRegistry } = await load('domains/cats/services/agents/invocation/InvocationRegistry'); + const { MessageStore } = await load('domains/cats/services/stores/ports/MessageStore'); + const { InMemoryTasteProposalStore } = await load('domains/taste/stores/InMemoryTasteProposalStore'); + const { callbackProposeTasteRoutes } = await load('routes/callback-propose-taste-routes'); + + const registry = new InvocationRegistry(); + const messageStore = new MessageStore(); + const store = new InMemoryTasteProposalStore(); + + const app = Fastify(); + await app.register(callbackProposeTasteRoutes, { + registry, + tasteProposalStore: store, + socketManager: socket(), + approvalIngress: new ApprovalIngress({ messageStore, socketManager: socket() }), + }); + await app.ready(); + + try { + const wake = await appendSchedulerWakeRow(messageStore, 'thread_1'); + const auth = await schedulerWokenAuth(registry, 'thread_1', wake.id); + + const res = await post(app, '/api/callbacks/propose-taste', auth, { + scene: 'A timer-driven turn tried to record a taste', + quote: 'The exemption must not follow the origin across producers', + tags: ['cognitive-honesty'], + dimension: 'cognitive-honesty', + privacy: 'public', + clientRequestId: 'f167-scheduler-origin-taste', + }); + + assert.ok(res.statusCode >= 500, `expected the ingress to reject, got ${res.statusCode}`); + assert.match( + res.body, + /owner mismatch/i, + 'must fail on the owner check specifically — any other 500 would pass this case for the wrong reason', + ); + for (const proposal of await store.listPending(OWNER)) { + assert.notEqual((await store.getPublication(proposal.id))?.state, 'anchored'); + } + } finally { + await app.close(); + } + }); +}); diff --git a/packages/api/test/propose-session-handoff-route.test.js b/packages/api/test/propose-session-handoff-route.test.js index 37c770da63..d0022cfa4b 100644 --- a/packages/api/test/propose-session-handoff-route.test.js +++ b/packages/api/test/propose-session-handoff-route.test.js @@ -281,4 +281,129 @@ describe('propose-session-handoff route (F225 ②a)', () => { const active = await ctx.handoffStore.listActiveBySession('sess_active'); assert.equal(active.length, 0, 'phantom proposal deleted after card-append failure'); }); + + // ── F167: the scheduler-origin path the catalog gate exists for ────────────────────── + // + // approval-ingress.test.js proves validateOrigin DECIDES correctly given a producerId + // and an origin row, but every case there synthesises the draft directly. None of them + // reaches the premise the design rests on: that THIS route derives threadId, messageId + // and ownerUserId from the authenticated InvocationRecord, which is the only reason a + // system pseudo-user row is safe to accept as an origin at all. + // + // `systemOriginExemption: 'server_attested'` is a hand-written DECLARATION. `required` + // stops an omission; it cannot stop a wrong one. So these cases test the BINDING. + // (maintainer gate 2 + @codex-luna P2 on PR #1349; tracked as #1350) + // + // Companion file: approval-hub/scheduler-origin-callback-integration.test.js walks the + // same wake row ACROSS producers (F128/F193/F231/F260 accept, F221 forbidden rejects). + // This block goes deep on the F225 route instead — the body-override case below is the + // one assertion neither that matrix nor the ingress unit suite makes. Neither file is + // redundant; delete either and a distinct failure mode stops being covered. + describe('scheduler-woken origin (F167)', () => { + async function proposeWithOrigin(ctx, { author, payload, threadId = 'thread_1' } = {}) { + const origin = await ctx.messageStore.append({ + ...author, + content: '⏰ 定时唤醒：持球检查 CI', + mentions: [], + timestamp: Date.now(), + threadId, + }); + // 7th arg is originTriggerMessageId: the EXACT turn origin, record-side only. + const auth = await ctx.registry.create('user_1', 'opus', 'thread_1', undefined, undefined, undefined, origin.id); + const response = await ctx.app.inject({ + method: 'POST', + url: '/api/callbacks/propose-session-handoff', + headers: { 'x-invocation-id': auth.invocationId, 'x-callback-token': auth.callbackToken }, + payload: payload ?? { done: 'held the ball through CI', nextSteps: 'read the verdict' }, + }); + return { origin, response }; + } + + // The reported defect (#1348): for any long-running session the message that triggered + // the invocation IS the scheduler wake row, so this was the one handoff that could never + // be proposed — 500 with no actionable text, while the same call from an A2A turn worked. + it('accepts a handoff anchored to the scheduler wake row', async () => { + const ctx = await buildCtx(); + const { origin, response } = await proposeWithOrigin(ctx, { author: { userId: 'scheduler', catId: null } }); + + assert.equal(response.statusCode, 200, 'scheduler-woken handoff is no longer rejected as a cross-tenant origin'); + const stored = await ctx.handoffStore.get(response.json().proposalId); + assert.equal(stored.status, 'pending'); + assert.deepEqual( + stored.publication.envelope.originRef, + { kind: 'message', threadId: 'thread_1', messageId: origin.id }, + 'origin stays anchored to the exact scheduler wake row', + ); + assert.equal(stored.publication.envelope.ownerUserId, 'user_1', 'owner is the record user, not the pseudo-user'); + }); + + it('also accepts the historical catId:"system" wake shape', async () => { + const ctx = await buildCtx(); + const { response } = await proposeWithOrigin(ctx, { author: { userId: 'system', catId: 'system' } }); + assert.equal(response.statusCode, 200, 'both persisted system shapes are accepted (visibility.ts predicate)'); + }); + + // The assertion that actually distinguishes a bound route from an unbound one. A status + // code cannot: a route that ignores these fields and a route that validates them both + // answer 200. Only the persisted originRef separates them. + it('the origin is record-derived — a body naming another thread/message/owner cannot move it', async () => { + const ctx = await buildCtx(); + const foreign = await ctx.messageStore.append({ + userId: 'user_2', + catId: null, + content: 'another tenant thread', + mentions: [], + timestamp: Date.now(), + threadId: 'thread_foreign', + }); + const { origin, response } = await proposeWithOrigin(ctx, { + author: { userId: 'scheduler', catId: null }, + payload: { + done: 'held the ball through CI', + nextSteps: 'read the verdict', + // None of these are inputs to the origin. If any ever becomes one, this test fails. + threadId: 'thread_foreign', + sourceThreadId: 'thread_foreign', + messageId: foreign.id, + sourceMessageId: foreign.id, + userId: 'user_2', + ownerUserId: 'user_2', + }, + }); + + assert.equal(response.statusCode, 200); + const envelope = (await ctx.handoffStore.get(response.json().proposalId)).publication.envelope; + assert.deepEqual( + envelope.originRef, + { kind: 'message', threadId: 'thread_1', messageId: origin.id }, + 'body-supplied thread/message are ignored; originRef is derived from the InvocationRecord', + ); + assert.equal(envelope.ownerUserId, 'user_1', 'body-supplied owner cannot reassign the proposal'); + }); + + // The exemption widens WHO may author a row inside an already-bound thread. It must not + // widen it to another human: cross-tenant isolation has to survive end to end, not just + // in the ingress unit case. + it('still rejects a foreign human origin in the same thread', async () => { + const ctx = await buildCtx(); + const { response } = await proposeWithOrigin(ctx, { author: { userId: 'user_2', catId: null } }); + + assert.equal(response.statusCode, 500, 'a non-system foreign author is still a cross-tenant origin'); + assert.match( + response.json().message ?? '', + /Approval origin message owner mismatch/, + 'fails for the tenancy reason, not some unrelated 500', + ); + }); + + // A system userId worn by a cat-authored row must not reach the exemption. Asserting the + // reason matters here too: this input is one typo in the predicate away from passing. + it('still rejects a system userId carried by a cat-authored row', async () => { + const ctx = await buildCtx(); + const { response } = await proposeWithOrigin(ctx, { author: { userId: 'scheduler', catId: 'opus' } }); + + assert.equal(response.statusCode, 500, 'isSystemUserMessage requires BOTH a system userId and a system/null catId'); + assert.match(response.json().message ?? '', /Approval origin message owner mismatch/); + }); + }); }); From 2b170e2b78f4528b6e294241880830a61036b2fe Mon Sep 17 00:00:00 2001 From: Ragdoll-Opus-5 Date: Thu, 13 Aug 2026 00:46:36 +0300 Subject: [PATCH 10/10] style(F167): biome format the two new test files CI Lint would have been red on both -- and the breadth file's violation was inherited, not introduced by the format pass, so running biome only on my own file would have shipped the red anyway. Caught locally before routing the review re-entry: a formatting red costs the reviewer a whole cycle to tell me something a 9ms local check knows. Co-Authored-By: Claude Opus 5 --- .../scheduler-origin-callback-integration.test.js | 3 ++- packages/api/test/propose-session-handoff-route.test.js | 6 +++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/api/test/approval-hub/scheduler-origin-callback-integration.test.js b/packages/api/test/approval-hub/scheduler-origin-callback-integration.test.js index 7406d6cb5e..be4af70aa7 100644 --- a/packages/api/test/approval-hub/scheduler-origin-callback-integration.test.js +++ b/packages/api/test/approval-hub/scheduler-origin-callback-integration.test.js @@ -106,7 +106,8 @@ describe('F167 scheduler-origin exemption over real callback routes', () => { socketManager: socket(), handoffProposalStore: store, sessionChainStore: { - getActive: async (catId, threadId) => (catId === session.catId && threadId === session.threadId ? session : null), + getActive: async (catId, threadId) => + catId === session.catId && threadId === session.threadId ? session : null, }, evidenceStore: { ingestRaw() {}, search: () => [] }, markerQueue: { enqueue() {} }, diff --git a/packages/api/test/propose-session-handoff-route.test.js b/packages/api/test/propose-session-handoff-route.test.js index d0022cfa4b..a59bffd34f 100644 --- a/packages/api/test/propose-session-handoff-route.test.js +++ b/packages/api/test/propose-session-handoff-route.test.js @@ -402,7 +402,11 @@ describe('propose-session-handoff route (F225 ②a)', () => { const ctx = await buildCtx(); const { response } = await proposeWithOrigin(ctx, { author: { userId: 'scheduler', catId: 'opus' } }); - assert.equal(response.statusCode, 500, 'isSystemUserMessage requires BOTH a system userId and a system/null catId'); + assert.equal( + response.statusCode, + 500, + 'isSystemUserMessage requires BOTH a system userId and a system/null catId', + ); assert.match(response.json().message ?? '', /Approval origin message owner mismatch/); }); });