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

[TERRYYYC]

The bug

propose_session_handoff returned 500 Approval origin message owner mismatch deterministically — three consecutive calls, not a transient fault.

The asymmetry is what made it look like a flaky server rather than a rule:

Trigger for the invocation	origin userId	handoff
scheduler wake row (持球唤醒)	scheduler	✗ 500
A2A cross-thread delivery	real owner	✓ works

So the sessions that need handoff most — the long-running ones, driven by timers, carrying the heaviest context — were the only ones that could never hand off.

Root cause

A handoff proposal anchors its approval card on the message that triggered the invocation:

// callback-propose-session-handoff-routes.ts
const originMessageId = record.originTriggerMessageId ?? record.a2aTriggerMessageId;

For a long-running session that message is the scheduler's wake row. Read back from the store before changing any code:

{ "userId": "scheduler", "catId": null, "threadId": "thread_mslv8bbw8pbazsz2" }

The thread belongs to default-user, so ApprovalIngress.validateOrigin passed the threadId assertion and then failed origin.userId !== draft.ownerUserId.

Why this is a bug, not the check working

The userId comparison was not protecting what it appeared to protect. Isolation on this path is a conjunction: the caller binds originRef to an authenticated InvocationRecord (fixing which thread may be named), and this class checks the stored origin is consistent with it. What the userId comparison additionally decided was only who may have authored a row inside an already-verified thread — and a system pseudo-user speaking in your own thread is not another tenant.

Fix

Reuse the store layer's existing predicate rather than deriving a second definition:

if (origin.userId !== draft.ownerUserId && !isSystemUserMessage(origin)) { throw ... }

isSystemUserMessage requires both a system userId (scheduler/system) and a system/null catId, so a cat-authored row wearing a system userId stays rejected. Two parallel definitions of "is this the system" is how they start to drift.

Tests — three, not one

The second and third exist so this fix cannot degrade into "delete the check":

1. 🔴→🟢 a scheduler-authored origin in the owner thread is accepted
2. 🟢 an origin authored by another human user is still rejected
3. 🟢 userId=scheduler carried by a cat-authored message is still rejected

RED was observed first, and confirmed red for the right reason:

✖ accepts a scheduler-authored origin in the owner thread
  Error: Approval origin message owner mismatch
      at ApprovalIngress.validateOrigin (ApprovalIngress.js:175:19)

Scope	Before	After
approval-ingress.test.js	15/16 (1 red)	16/16
approval-hub/*.test.js	—	299/299
session-handoff-* + propose-session-handoff-route	—	54/54

The pre-existing rejects ... cross-owner origins case stays green throughout — direct evidence the cross-user protection was not weakened.

Review provenance

Reviewed by @codex-luna (Maine Coon — cross-family, non-author) across four rounds. Final verdict on e5b6b1c96: APPROVE, no P1/P2/P3, with independent verification (build, 60/60, 299/299, F225 route 10/10, git diff --check) and a mechanical non_comment_changed_lines=0 check on the comment-only increments.

Commits are not amended — each of the reviewer's verdicts stays anchored to the SHA it was given for:

• 7d4870898 fix + tests — APPROVE
• 7843fb6fa boundary comment — APPROVE + P3
• 2bad996bc review credit + scope — APPROVE + P3
• bd609336d caller-invariant attribution — APPROVE + P3
• e5b6b1c96 isolation stated once as a conjunction — APPROVE, converged

Three of those rounds landed on the same sentence. The reason was 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 corrected the quoted instance and left its paraphrases standing. The final commit states it once, as numbered parts, so there is no second site for it to drift.

Scope

Behaviour change is one condition in ApprovalIngress.validateOrigin. No schema migration, no new wire, no external contract change. The other four commits are comment-only.

Merge is not authorized by this author line — merge-gate is still pending.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

To use Codex here, create an environment for this repo.

[zts212653]

Thanks for the detailed reproduction and the focused tests. The timer-wake failure mode is plausible, but this PR is not ready to enter formal merge review yet.

Two direction gates need to be closed first:

1. Please open or link a clowder-ai issue for this bug, including the exact reproduction, expected behavior, and how the scheduler-authored wake row is produced. Our inbound process requires an accepted issue before a new community patch enters merge review.
2. The patch relaxes ApprovalIngress.validateOrigin() for every message-origin producer, while the security argument in the code only establishes authenticated origin binding for the F225 session-handoff caller. Please either scope the exemption to a server-attested/F225 origin, or prove that every producer which can reach this shared ingress establishes the same owner/thread/message binding. An integration test from the authenticated F225 callback record through ApprovalIngress would make that boundary concrete.

One related correction: the new unit-test comment says the threadId equality check alone pins the origin to the caller's own thread. It does not; the production comment correctly notes that this only becomes a tenancy boundary when combined with authenticated caller binding. Please make those explanations consistent.

I’m marking this needs-info for now. Once the issue is linked and the shared-boundary scope is resolved, we can continue with exact-HEAD review. The implementation remains with you; we are not taking over the branch.

[TERRYYYC]

Per maintainer direction, this work is being tracked in #1348 first. Closing this PR without merging; the branch and commits remain available for the issue workflow.