Name a Signal reaction's target by author and sent timestamp on the v2 outbox path

[MaxGhenis]

Stacked on #225 (claude/signal-v2-quote-replies, head e0ad2851), which this builds on. Merge that first; this PR's own change is the single commit on top.

What was wrong

A Signal reaction dispatched through the durable outbox took this path: dispatchReactionLease → targetRefForLease (internal/messaging/dispatch.go) → bridge.ReactionRequest.Target → Signal adapter SendReaction → signallive.Bridge.SendReactionRequest, which ran

signal-cli sendReaction -a <author> -t <v2 remote_message_id>

signal-cli's -t is the target message's sent timestamp. The v2 remote ID is that timestamp only for this account's own sends. An incoming message's is a SHA-1 (v2keys.SignalIncomingSourceID, signaldecoder.go), and a migrated own message can carry local:<sha1>.

Confirmed before changing anything

Two tests written against the unchanged code (e0ad2851), both failing:

--- FAIL: TestReproSendReactionRequestPassesV2RemoteIDToTargetTimestamp
    signal-cli -t = "36c1e3741b6432157a0af5fe136a306fd941ee21" (the v2 remote ID), want the sent timestamp 1700000000123
--- FAIL: TestV2ReactionToIncomingSignalMessageTargetsItsSentTimestamp
    signal-cli sendReaction calls = [["-a" "+15551230000" "sendReaction" "-e" "👍" "-a" "+15551234567" "-t" "36c1e3741b6432157a0af5fe136a306fd941ee21" "+15551234567"]],
    want one [... "-t" "1700000000123" "+15551234567"]

The first calls SendReactionRequest with the runSignalCLI stub. The second ingests a real receive line through the v2 decoder and worker, submits a reaction to MessageService, and runs DispatchDue through the real Signal adapter and bridge. It is kept in this PR and now passes. The first is replaced by the rule and argv tables below, because the function's signature changed.

What signal-cli does with that argument, run against an empty config directory (signal-cli 0.14.8):

signal-cli: error: argument -t/--target-timestamp: could not convert
'36c1e3741b6432157a0af5fe136a306fd941ee21' to integer (64 bits)        (exit 1)

I then ran the old code with a stub that fails that way. The outbox row ended uncertain (class=transient code=send_reaction): the failure is a CommandError without the not-dispatched marker, so the adapter leaves Dispatch unset and recordSendError falls through to uncertain. Nothing is sent and nothing retries.

How far this reaches today

Nothing in production submits a reaction to the v2 outbox yet, so this is a latent bug, not a live one. MessageService.SendReaction has no caller outside tests. The web UI's /api/react and the react_to_message MCP tool both call App.SendSignalReaction → the legacy signallive.Bridge.SendReaction, which looks the target up in the legacy store by message ID. That path is untouched here. This fix matters when reactions are routed through the outbox.

The fix

A reaction carries nothing but the name of its target: author and sent timestamp. If either is wrong, signal-cli still gets a well-formed command, aimed at a message that does not exist. So the transport now sends a reaction only when the store vouches for both, and stops before signal-cli otherwise.

Target	-a	-t
Incoming, sender stored, remote ID a SHA-1	the stored sender	occurred_at_ms. A SHA-1 is the ID a Signal receiver gives a message it keyed by sender and sent timestamp, and each writer that uses it stores that timestamp as the occurred time: the v2 decoder, the legacy receiver, a Signal Desktop import of a row with a sent time. The ID itself is not sent.
Incoming, any other remote ID		refused: signal reaction target timestamp is unavailable
Incoming, no sender stored	refused: signal reaction target author is unavailable
This account's, remote ID a positive decimal	this account	the remote ID. That is how a send Signal accepted is stored when its timestamp was kept: an outbox confirm, a sync message from the phone, the legacy SendMedia, the legacy-primary projector.
This account's, any other remote ID		refused: signal reaction target timestamp is unavailable

Where	Change
bridge.MessageRef	Adds Outgoing: this account sent the message. An empty AuthorID cannot say that, because incoming messages can lack a sender too. The doc comment says so.
messaging.targetRefForLease, replyRefForLease	Set Outgoing from the stored direction. Nothing else changes in the dispatcher.
signallive.ReactionTargetArgs (new, pure, reaction_target.go)	The rule in the table.
Bridge.SendReactionRequest	Takes a signallive.ReactionTarget{RemoteID, AuthorID, Outgoing, SentAt} instead of two strings, and fails before running signal-cli on a target it cannot name.
Signal adapter	ReactionTarget(bridge.MessageRef) hands the dispatcher's description to signallive. ReplyTarget carries Outgoing too (#225's reflection test requires every MessageRef field); QuoteArgs does not read it, and quote behaviour is unchanged.
v2keys	IsSignalIncomingSourceID (40 lowercase hex digits) and SignalReceivedSourceID (received:<sha1>).
Signal Desktop importer	An incoming row with no sent time is stored under the time it was received, as before, but its source ID is now received:<sha1> instead of a plain SHA-1 of that time. Its message ID is unchanged, so a re-import rewrites the row an earlier import stored instead of adding one. The migration carries the source ID into v2 as the remote ID.
docs/agent-runbook.md	New "Signal reactions through the v2 outbox" section, including how to read a refused row.

A refused reaction is a plain error, not a CommandError, so the adapter reports it as not dispatched and the outbox retries it. A reaction to this account's own pending send therefore goes out once the send is confirmed and its remote ID becomes Signal's timestamp.

This is not the rule you asked for, and why

You asked for #225's quote rule: occurred_at_ms, except a decimal remote ID for a self-authored target. My first version did that (plus a dispatcher guard for unsent sends). Two rounds of independent review (GPT-6.1 Sol; it could read but not run tests) each requested changes, and each time the reviewer was right.

Round 1. Three stored shapes for which that rule sends a well-formed, wrong target. I reproduced all of them against that version before changing it. Each ran this, for a message with no such identity:

signal-cli ran [["-a" "+15551230000" "sendReaction" "-e" "👍" "-a" "+15551230000" "-t" "1700000000500" "+15551234567"]] for a target v2 cannot name

1. A migrated scheduled send caught in sending is imported with a derived request ID, its creation time, and no outbox row (migration/transform.go). My guard looked for the outbox row, found none, and the creation time went out as -t.
2. An incoming message with no stored sender has an empty AuthorID, which was read as this account. The legacy receiver stored a group message with no source that way (handleDataMessage returns early only when both the source and the group are empty), migration keeps it senderless, and the legacy-primary mirror records no sender at all. My "I know of no Signal row like that" in the first description was wrong.
3. A migrated own local:<sha1> row's time is not always Signal's. The legacy Bridge.SendText sets timestamp := now().UnixMilli() after signal-cli returns and derives the alias from it. The phone's own sends were stored under the same alias with the right time, and nothing in v2 tells the two apart.

The premise of the fallback, that an own message's occurred time is its Signal timestamp, fails for all three. So for a message this account sent, only a decimal remote ID is trusted. The dispatcher guard is gone: the transport rule does not need it, and it missed the first shape.

Round 2. The round-1 findings were confirmed resolved, with one new one: the same kind of hole on the incoming side, which I had listed as a known gap instead of fixing. signalDesktopMessageTimestamp falls back to received_at when a Signal Desktop row has no sent_at, and the importer keyed that row by a plain SHA-1 of the received time: indistinguishable from a message keyed by its sent timestamp, so a reaction would name the received time. I reproduced it: with the importer's marker in place but the rule still trusting any incoming ID (mutant N15), the new importer-to-transport test reports

reaction to the row without a sent time = ["-a" "+15551234567" "-t" "1700000000500"], <nil>; want it refused, not sent with the received time

The fix is at the writer and in the rule: the importer marks such a row's source ID, and an incoming target is named only under a receiver's SHA-1.

Quotes keep #225's more lenient rule. A quote carries the quoted text; a reaction carries nothing else. The same shapes reach QuoteArgs, which I have not changed; I've noted them on #225.

Invariants (tested)

Transport (TestReactionTargetArgsInvariants: 5,000 seeded random targets against an independently written oracle; the generator must produce at least 150 own and 150 incoming successes, 200 author failures and 500 timestamp failures)

• A1 shape: success is exactly -a <non-empty> -t <positive decimal>.
• A2 own message: the author is the account and the timestamp is the remote ID when it is a positive int64 decimal. Any other remote ID fails as timestamp unavailable, whatever SentAt holds.
• A3 incoming message: the author is the trimmed AuthorID and the timestamp is SentAt, and only when the remote ID is 40 lowercase hex digits. No AuthorID fails as author unavailable; any other remote ID, or a zero or non-positive SentAt, fails as timestamp unavailable.
• A4, from A2 and A3: -t is never a remote ID other than a decimal one of an own message, never the stored time of a message whose ID does not vouch for it, and -a is never the account for an incoming message that names no sender.
• A5 agreement with quotes: wherever a reaction names a message, QuoteArgs names the same author and timestamp for it (one shape excluded and labelled: an incoming message stored under this account's own address, which QuoteArgs reads as an own message).
• A6 determinism.

Dispatcher (TestReactionTargetRefInvariants: 40 seeds of random conversations mixing incoming messages with and without a sender and with SHA-1 or decimal IDs, and outgoing text and media sends that were confirmed, canceled, or still scheduled; each reaction is mapped twice by forcing one not-dispatched retry)

• D1 target: the ref names the target's current remote ID (its request ID until the transport confirms it).
• D2 direction: Outgoing is set exactly for a message this account sent, whether or not the message names a sender.
• D3 fidelity: AuthorID is the sender's canonical value ("" for none) and SentAt the occurred time.
• D4: the reply-only fields stay empty.
• D5 determinism: a retried attempt carries the same ref.

Writer to transport

• W1 Signal Desktop import → migration → dispatcher → transport (TestSignalDesktopRowWithoutASentTimeCannotBeAReactionTarget, in the importer package so it runs the real importer with a stubbed export). Two incoming rows go through ImportFromDB, migration.Transform, a real MessageService reaction and the adapter conversion into ReactionTargetArgs. The row with a sent time is named -a <peer> -t <sent time>. The row without one reaches the transport as the peer's incoming message at the received time under received:<sha1>, and is refused.
• W2 a re-import marks in place (TestSignalDesktopReimportMarksARowStoredBeforeTheMarker): a row an earlier import stored under the unmarked hash ends as the same single row with the marked source ID.
• W3 SignalReceivedSourceID never satisfies IsSignalIncomingSourceID (v2keys tests).

End to end

• E1 differential, new path = legacy path where both can name the message (TestV2ReactionMatchesLegacyReaction). It reuses Quote Signal replies from the v2 message on v2-primary #225's corpus: 12 seeds of random signal-cli receive lines in eight shapes (incoming from E.164, from a known ACI, from an unknown ACI, in a group, attachment-only, text plus attachment, sync-sent text, sync-sent attachment), each fed through both the legacy receive handler and the v2 decoder and worker. For every message both stores hold (127, every shape covered) it sends the same random reaction (emoji × add/remove/switch) down both paths and compares the signal-cli argv: the legacy SendReaction on the legacy row, and a reaction submitted to MessageService and dispatched through the real Signal adapter to a bridge whose legacy store is empty. The argv are identical. This cannot detect a stored timestamp that both paths share and that is wrong, which is what both reviews found.
• E2 nothing is sent for a target v2 cannot name (TestV2ReactionRefusesATargetItCannotName): seven stored shapes, each imported into a v2 store and reacted to through the real adapter and bridge: migrated sending; migrated local: alias; incoming with no sender under a SHA-1, a decimal ID, and the mirror's signal:<decimal>; a peer's Desktop row under received:<sha1>; a peer's message under a decimal ID. Zero signal-cli calls, the row not_dispatched / transient, and the recorded error_detail names the right reason.
• E3 a pending own send (TestV2ReactionToOwnPendingSendWaitsForItsTransportTimestamp): zero calls while the send is scheduled; after it is confirmed, exactly one sendReaction, with -t equal to the timestamp signal-cli returned for the send.
• L1 the legacy path is unchanged. Bridge.SendReaction is not in the diff, and its characterization test still passes.
• L2 quotes are unchanged. reply_quote.go differs from Quote Signal replies from the v2 message on v2-primary #225 by one struct field and its doc comment; Quote Signal replies from the v2 message on v2-primary #225's quote tests pass unmodified apart from one fixture line.

Tests

• internal/signallive/reaction_target_test.go: a 22-row rule table; the A1–A6 property test; signal-cli argv through the runSignalCLI stub for each kind of target, checking that the five refused kinds run no command and return a plain error.
• internal/signallive/reaction_target_external_test.go: the dispatcher-level reproduction, E1, E2 and E3.
• internal/importer/signal_desktop_reaction_test.go: W1 and W2. internal/v2keys/derive_test.go: W3 and the recogniser's table.
• internal/bridgeadapters/signal/reaction_target_test.go: a reflection check that every ReactionTarget field is the MessageRef field of the same name and that the set is RemoteID, AuthorID, Outgoing, SentAt; SendReaction hands the description to the poller; the real refusal errors are classified transient and not dispatched, with no lifecycle transition.
• internal/messaging/reaction_ref_test.go (through MessageService.DispatchDue with the scripted registry): a senderless incoming target and an own target differ only in Outgoing; a reaction to an own scheduled send carries its request ID, then the transport's ID after the send confirms; described reply refs carry Outgoing; D1–D5.
• cmd/r5_signal_reaction_test.go, on Quote Signal replies from the v2 message on v2-primary #225's migrated v2-primary harness with a reaction-capable scripted Signal account: a reaction to a live-ingested incoming message (SHA-1 remote ID → the decoder's timestamp and the ACI author); to this account's own outbox send (signal-cli's timestamp, not the stored submit time); and three refusals on the real migration output: the fixture's incoming row under a decimal ID, its local:abc123r5, and its scheduled send caught in sending.
• Existing SendReactionRequest tests in client_test.go and the adapter's mapping test now pass described targets. One line of Quote Signal replies from the v2 message on v2-primary #225's reply_target_test.go fixture sets the new field.

Run locally, with a private build cache: go build ./..., go vet ./..., and the full signallive, bridgeadapters/..., messaging, bridge, importer, migration and v2keys packages plus the TestR5* tests in cmd all pass; the new tests also pass under -race. I did not run go test ./... locally (the host was short on disk); CI does.

Mutation check

Each mutant was applied alone and the targeted tests rerun.

All eighteen were caught.

Mutant	Tests that failed
N1 an incoming target's -t is its remote ID (the original bug)	10, including E1, E2, W1 and the R5 test
N2 an own target falls back to its occurred time (this PR's first version)	7, including E2, E3 and the R5 test
N3 an incoming target prefers a decimal remote ID	rule table, A3, E2, R5
N4 an empty author means this account	rule table, A3, argv table, E2
N5 the dispatcher never says Outgoing	7, including D2, E1, E2 (wrong reason recorded), E3
N6 the dispatcher always says Outgoing	7, including D2, E1, E2, W1
N7 the adapter drops Outgoing	6, including the reflection check, E1, E2, E3
N8 the adapter drops SentAt	7, including the reflection check, E1, W1
N9 SendReactionRequest ignores the refusal	argv table, E2, E3
N10 a reply ref never says Outgoing	the reply-ref direction test
N11 the reply conversion drops Outgoing	#225's reflection test
N12 an own target keeps a stored author	rule table, A2
N13 the adapter drops AuthorID	8, including the reflection check, E1, E2, W1
N14 the importer does not mark a row with no sent time (round 2, writer side)	W1, W2
N15 an incoming target is trusted under any remote ID (round 2, transport side)	rule table, A3, argv table, W1, E2, R5
N16 any 40 characters pass for a receiver's ID	the recogniser's table, rule table, A3
N17 the importer marks rows that have a sent time	W1
N18 the marker is a bare hash	W3, rule table, argv table, W1, E2

Behaviour changes to be aware of

• Own messages stored under a non-decimal ID cannot be reacted to through the outbox. That is every local:<sha1> row: what the legacy SendText sent, and what was sent from the phone, before the v2 cutover. Some carry Signal's timestamp (the phone's) and some the wall clock (SendText's), and v2 cannot tell which. Refusing all of them is the cost of never sending a reaction that silently names nothing. Own messages stored under signal-cli's timestamp still work (legacy SendMedia, the legacy-primary projector, and everything sent or synced since cutover). On this path those reactions failed before this PR too (-t local:…). If you would rather send them and accept that some are lost, it is one branch in ReactionTargetArgs; mutant N2 is exactly that change.
• The Signal Desktop importer writes a different source ID for an incoming row with no sent time (received:<sha1>), and a re-import rewrites the source ID of such a row stored earlier. Its message ID, timestamp and everything else are unchanged.
• SendReactionRequest no longer reads an empty author as this account. The dispatcher says Outgoing for every own message, and the adapter is the function's only caller.
• A reaction that can never be named stays not_dispatched and retries every 5 s (defaultRetryDelay), which has no cap on this branch. The open Stop Google sends sticking on 'no conversation': detect the account-pairing switch, bound retries, and show refused sends #204 proposes a general retry budget for such rows; this PR adds none of its own.

Not covered

• A v2 store migrated before this change can still hold a Signal Desktop row with no sent time under an unmarked SHA-1. Nothing in either store distinguishes it from a message keyed by its sent timestamp, so a reaction to it names the received time. A re-import fixes the legacy row; nothing here re-keys the v2 copy. I do not know whether Signal Desktop ever leaves sent_at empty for a message row; the exporter and importer are written as if it can.
• An incoming placeholder the legacy receiver stored for an edit whose original it never saw (missing-edit:<sha1>) is refused, because its ID is not a receiver's SHA-1. Round 2's reviewer traced its stored time to the original's sent timestamp; I did not widen the rule for it.
• The Desktop exporter also takes rows of non-standard types (text, story-reply, empty) when they have a body, and the importer treats anything not typed outgoing as incoming. If Signal Desktop stores a message this account wrote under such a type, it is imported as the peer's. The reviewer raised this and could not establish such a row; neither can I. Unchanged here.
• The WhatsApp adapter also reads an empty AuthorID as this account (whatsapplive.SendReactionRequest). Outgoing is there for it to use; I did not change it or check whether WhatsApp has senderless incoming rows.
• Reactions from the UI on a v2-primary install. By code reading, not reproduced: /api/react has no V2Primary branch and routes by a signal:/whatsapp: prefix or a legacy-store lookup, while the UI on v2-primary sends v2 conversation and message IDs. That is a separate change (route reactions through the outbox), flagged as a follow-up task; it is also what would make this fix reachable.

🤖 Generated with Claude Code