Skip to content

Commit e08405d

Browse files
committed
gateway: review fixes — refuse unresolvable projects, durable one-shot TTS, uuid Signal principals
P1: a queued task whose snapshotted project was disabled or deleted before execution is now REFUSED with a durable reply — ResolveProject failure no longer falls back to the gateway default root, closing the contradiction with the 'never helpfully run elsewhere' claim. All refusal paths share one helper; regression test added. P2 (TTS): voice replies are synthesized exactly ONCE, at job completion, and the spool ID is persisted on the replied row — a delivery retry or a restart re-sends the same file instead of re-billing TTS ('always') or losing the in_kind decision (attachments aren't loaded by the replay). P2 (Signal): the principal is now the account UUID — Signal's stable identity — with the phone number only as fallback; numbers change hands. Pairing keeps uuids painless to allow-list. P2/P3 (email): the From-address identity caveat is now stated in the package doc and both docs (allow-list strength depends on the provider's SPF/DKIM/DMARC filtering; dedicated mainstream account advised); explicit Authentication-Results enforcement is a tracked follow-up.
1 parent 95895fe commit e08405d

11 files changed

Lines changed: 128 additions & 55 deletions

File tree

‎docs/gateway/README.md‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,10 @@ Webhook-driven surfaces (Teams, Google Chat, SMS, GitHub, WhatsApp) mount on
5050
the shared listener (`webhook.addr`, default `:8787`) at
5151
`/webhook/{teams,googlechat,sms,github,whatsapp}` — expose it over HTTPS.
5252
Email dedup is keyed on `<mailbox>/<UIDVALIDITY>/<UID>` (the provider-side ack
53-
identity); Message-ID serves threading only. Signal requires a signal-cli
53+
identity); Message-ID serves threading only. Email's sender identity is the
54+
RFC From address — weaker than the other channels' platform-verified ids, so
55+
its allow-list depends on your mailbox provider rejecting spoofed mail
56+
(SPF/DKIM/DMARC); use a mainstream provider and a dedicated account. Signal requires a signal-cli
5457
daemon in native HTTP mode; Matrix v1 is plain rooms only (E2EE is a known
5558
follow-up).
5659

‎internal/channels/email/email.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,14 @@
99
// re-fetches it. The durable dedup key is <mailbox>/<UIDVALIDITY>/<UID> — the
1010
// provider-side identity, robust against malformed or duplicated Message-IDs
1111
// (which serve threading, not dedup).
12+
//
13+
// IDENTITY CAVEAT: the principal is the RFC From address — weaker than the
14+
// other channels' platform-authenticated ids, since From can be spoofed by
15+
// mail that evades the provider's SPF/DKIM/DMARC filtering. The mailbox
16+
// provider's authentication is the real gate (a mainstream provider rejects or
17+
// junks spoofed mail before we poll it), which is one more reason for the
18+
// dedicated-account model. Enforcing Authentication-Results=pass explicitly is
19+
// a tracked follow-up.
1220
package email
1321

1422
import (

‎internal/channels/signal/signal.go‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -174,13 +174,15 @@ func parseEnvelope(raw []byte, account string) (channels.Inbound, []attachmentRe
174174
if dm == nil {
175175
return channels.Inbound{}, nil, false // receipt/typing/sync — not a message
176176
}
177-
// Principal: the E.164 number when known (the identity users recognize and
178-
// allow-list), the uuid otherwise. Our own messages are skipped (loops).
179-
principal := env.SourceNumber
177+
// Principal: the account UUID — Signal's STABLE identity. A phone number can
178+
// change hands or be re-registered, so it is only the fallback when the
179+
// daemon didn't surface a uuid. The pairing flow makes uuids painless to
180+
// allow-list (nobody has to type one). Our own messages are skipped (loops).
181+
principal := env.SourceUUID
180182
if principal == "" {
181-
principal = env.SourceUUID
183+
principal = env.SourceNumber
182184
}
183-
if principal == "" || principal == account {
185+
if principal == "" || env.SourceNumber == account || principal == account {
184186
return channels.Inbound{}, nil, false
185187
}
186188
var refs []attachmentRef

‎internal/channels/signal/signal_test.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,14 @@ func TestParseEnvelopeDM(t *testing.T) {
1919
if !ok {
2020
t.Fatal("parse failed")
2121
}
22-
if inb.Channel != "signal" || inb.Principal != "+15551230000" || inb.Conversation != "+15551230000" {
22+
// Principal is the STABLE uuid; the phone number is only a fallback.
23+
if inb.Channel != "signal" || inb.Principal != "uuid-1" || inb.Conversation != "uuid-1" {
2324
t.Errorf("inbound = %+v", inb)
2425
}
2526
if !inb.IsDirect || inb.Mentioned {
2627
t.Errorf("gating = %+v", inb)
2728
}
28-
if inb.MessageID != "+15551230000:1700000000001" {
29+
if inb.MessageID != "uuid-1:1700000000001" {
2930
t.Errorf("MessageID = %q (dedup key is sender:timestamp)", inb.MessageID)
3031
}
3132
if len(refs) != 0 {

‎internal/gateway/server/media.go‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,11 @@ func newSpeaker() speaker {
5252
}
5353

5454
// maybeSpeak synthesizes a voice rendition of a reply when the channel's
55-
// voice_replies policy asks for one, returning the spool path ("" = text
56-
// only). Policy: "always", or "in_kind" when the task arrived with a voice
57-
// note. Failures degrade silently to text — a reply is never lost to TTS.
55+
// voice_replies policy asks for one, returning the SPOOL ID ("" = text only) —
56+
// the durable handle that rides the replied row, so retries and restarts
57+
// re-send the same file instead of re-billing TTS. Policy: "always", or
58+
// "in_kind" when the task arrived with a voice note. Failures degrade silently
59+
// to text — a reply is never lost to TTS.
5860
func (r *runtime) maybeSpeak(ctx context.Context, it state.Item, reply string) string {
5961
if r.tts == nil {
6062
return ""
@@ -87,7 +89,7 @@ func (r *runtime) maybeSpeak(ctx context.Context, it state.Item, reply string) s
8789
if err != nil {
8890
return ""
8991
}
90-
return att.Path
92+
return att.ID()
9193
}
9294

9395
// spokenSummary renders a reply as speakable text: code blocks dropped (nobody

‎internal/gateway/server/media_test.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"strings"
99
"testing"
1010

11+
"github.com/memcode-ai/memcode/internal/channels"
1112
gwconfig "github.com/memcode-ai/memcode/internal/gateway/config"
1213
"github.com/memcode-ai/memcode/internal/gateway/state"
1314
)
@@ -79,7 +80,10 @@ func TestMaybeSpeakPolicy(t *testing.T) {
7980
if p == "" || tts.called != 1 {
8081
t.Fatalf("in_kind with voice note: %q %d", p, tts.called)
8182
}
82-
if _, err := os.Stat(p); err != nil {
83+
// The return is a durable spool ID, not a path — it must resolve in the spool.
84+
if resolved, err := channels.ResolveSpoolID(dir, p); err != nil {
85+
t.Errorf("voice id %q must resolve in the spool: %v", p, err)
86+
} else if _, err := os.Stat(resolved); err != nil {
8387
t.Errorf("voice file missing: %v", err)
8488
}
8589
rt.settings = gwconfig.Settings{Channels: map[string]gwconfig.Channel{"telegram": {VoiceReplies: "always"}}}

‎internal/gateway/server/reply_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ func TestDeliverReplySurvivesSendFailure(t *testing.T) {
3535

3636
it := state.Item{Channel: "telegram", MessageID: "m1", Conversation: "42", Principal: "p", Text: "hi"}
3737
gw.Accept(ctx, it, time.Unix(1000, 0))
38-
if err := gw.SetReplied(ctx, "telegram", "m1", "the answer"); err != nil {
38+
if err := gw.SetReplied(ctx, "telegram", "m1", "the answer", ""); err != nil {
3939
t.Fatal(err)
4040
}
4141

‎internal/gateway/server/selection_test.go‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ package server
33
import (
44
"context"
55
"io"
6+
"strings"
67
"testing"
8+
"time"
79

810
"github.com/memcode-ai/memcode/internal/channels"
911
gwconfig "github.com/memcode-ai/memcode/internal/gateway/config"
@@ -112,3 +114,41 @@ func TestChannelProjectPolicy(t *testing.T) {
112114
t.Errorf("resolveSelection default = %q, want www (channel policy)", p)
113115
}
114116
}
117+
118+
// A queued task whose snapshotted project is disabled or deleted before
119+
// execution is REFUSED — never run in the gateway default root (P1 from the
120+
// channels-baseline review).
121+
func TestRunJobRefusesUnresolvableProject(t *testing.T) {
122+
ctx := context.Background()
123+
gw, err := state.Open(ctx, t.TempDir())
124+
if err != nil {
125+
t.Fatal(err)
126+
}
127+
defer gw.Close()
128+
129+
sender := &capturingSender{}
130+
rt := &runtime{
131+
root: t.TempDir(),
132+
gw: gw,
133+
settings: gwconfig.Settings{
134+
// The id is allowed on the channel, but the project itself is disabled.
135+
Projects: map[string]gwconfig.Project{"www": {Path: t.TempDir(), Enabled: false}},
136+
},
137+
mediaDir: t.TempDir(),
138+
byName: map[string]replySender{"telegram": sender},
139+
out: io.Discard,
140+
notify: make(chan struct{}, 1),
141+
}
142+
it := state.Item{Channel: "telegram", MessageID: "m1", Conversation: "1", Principal: "me", Text: "do it", Project: "www"}
143+
if _, err := gw.Accept(ctx, it, time.Now()); err != nil {
144+
t.Fatal(err)
145+
}
146+
rt.runJob(ctx, it)
147+
if !strings.Contains(sender.last, "no longer available") {
148+
t.Fatalf("want refusal reply, got %q", sender.last)
149+
}
150+
// The refusal is durable: the item moved past pending without spawning.
151+
if p, _ := gw.Pending(ctx); len(p) != 0 {
152+
t.Errorf("item still pending: %+v", p)
153+
}
154+
}

‎internal/gateway/server/server.go‎

Lines changed: 38 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -419,28 +419,22 @@ func (r *runtime) runJob(ctx context.Context, it state.Item) {
419419
cfg := settings.Get(it.Channel)
420420
session := conversationSession(it.Channel, it.Conversation, it.Agent)
421421
// Resolve the snapshotted project id to its canonical root. The registry plus
422-
// the channel's project policy is the authorization boundary: an id that no
423-
// longer resolves — or that the channel is no longer allowed to use — falls
424-
// back to the gateway default rather than executing somewhere unauthorized.
422+
// the channel's project policy is the authorization boundary, re-checked at
423+
// EXECUTION, not only at snapshot: a task whose project was disallowed,
424+
// disabled, or deleted while queued is REFUSED — never "helpfully" run in the
425+
// gateway default root instead.
425426
root := r.root
426427
if it.Project != "" {
427-
// The channel's project policy is re-checked at execution, not only at
428-
// snapshot: if it tightened while this task was queued, refuse — never
429-
// "helpfully" run the task somewhere the channel wasn't pointed.
430428
if !settings.ProjectAllowed(it.Channel, it.Project) {
431-
msg := fmt.Sprintf("Project %q is not allowed on this channel anymore; nothing was run.", it.Project)
432-
if serr := r.gw.SetReplied(ctx, it.Channel, it.MessageID, msg); serr != nil {
433-
fmt.Fprintf(r.out, "gateway: recording policy refusal for %s: %v\n", it.Channel, serr)
434-
return
435-
}
436-
r.deliverReply(ctx, it, msg)
429+
r.refuse(ctx, it, fmt.Sprintf("Project %q is not allowed on this channel anymore; nothing was run.", it.Project))
437430
return
438431
}
439-
if resolved, rerr := settings.ResolveProject(it.Project); rerr == nil {
440-
root = resolved
441-
} else {
442-
fmt.Fprintf(r.out, "gateway: project %q for %s no longer resolves (%v); using default\n", it.Project, it.Channel, rerr)
432+
resolved, rerr := settings.ResolveProject(it.Project)
433+
if rerr != nil {
434+
r.refuse(ctx, it, fmt.Sprintf("Project %q is no longer available (%v); nothing was run.", it.Project, rerr))
435+
return
443436
}
437+
root = resolved
444438
}
445439
// Voice notes are transcribed HERE — after the durable record, before the
446440
// spawn — so the transcript becomes task text and audio never reaches the
@@ -449,12 +443,7 @@ func (r *runtime) runJob(ctx context.Context, it state.Item) {
449443
// an empty task.
450444
task, rest, sttMissing := r.transcribeAudio(ctx, it.Text, it.Attachments)
451445
if strings.TrimSpace(task) == "" && sttMissing {
452-
msg := "Voice note received, but no transcription provider is configured. Set OPENAI_API_KEY or GEMINI_API_KEY on the gateway machine, or send text."
453-
if serr := r.gw.SetReplied(ctx, it.Channel, it.MessageID, msg); serr != nil {
454-
fmt.Fprintf(r.out, "gateway: recording voice-note refusal for %s: %v\n", it.Channel, serr)
455-
return
456-
}
457-
r.deliverReply(ctx, it, msg)
446+
r.refuse(ctx, it, "Voice note received, but no transcription provider is configured. Set OPENAI_API_KEY or GEMINI_API_KEY on the gateway machine, or send text.")
458447
return
459448
}
460449
it.Text = task
@@ -471,12 +460,7 @@ func (r *runtime) runJob(ctx context.Context, it state.Item) {
471460
if err != nil {
472461
// A spawn failure won't succeed on replay; record the error as the reply so
473462
// it rides the same durable delivery path instead of being lost.
474-
msg := "Couldn't start that: " + err.Error()
475-
if serr := r.gw.SetReplied(ctx, it.Channel, it.MessageID, msg); serr != nil {
476-
fmt.Fprintf(r.out, "gateway: recording spawn failure for %s: %v\n", it.Channel, serr)
477-
return
478-
}
479-
r.deliverReply(ctx, it, msg)
463+
r.refuse(ctx, it, "Couldn't start that: "+err.Error())
480464
return
481465
}
482466
r.event(ctx, events.KindGatewayJobSpawned, eventPayload{Channel: it.Channel, Conversation: it.Conversation, PrincipalID: it.Principal, MessageID: it.MessageID, JobID: job.ID})
@@ -486,16 +470,33 @@ func (r *runtime) runJob(ctx context.Context, it state.Item) {
486470
if strings.TrimSpace(reply) == "" {
487471
reply = "Done."
488472
}
473+
// Synthesize any voice rendition ONCE, here — before the durable handoff — so
474+
// a delivery retry or a restart re-sends the same spool file instead of
475+
// re-billing TTS or losing the in_kind decision (the pending-replies replay
476+
// carries the voice spool ID, not the attachment list).
477+
voice := r.maybeSpeak(ctx, it, reply)
489478
// Durable handoff: the job is finished and must never re-run, even if delivery
490479
// below fails or the process crashes. From here the reply is the worker's to
491480
// deliver. A rare DB write failure leaves the item pending and re-runs it.
492-
if err := r.gw.SetReplied(ctx, it.Channel, it.MessageID, reply); err != nil {
481+
if err := r.gw.SetReplied(ctx, it.Channel, it.MessageID, reply, voice); err != nil {
493482
fmt.Fprintf(r.out, "gateway: recording reply for %s failed: %v\n", it.Channel, err)
494483
return
495484
}
485+
it.Voice = voice
496486
r.deliverReply(ctx, it, reply)
497487
}
498488

489+
// refuse records msg as the task's durable reply (no job runs, no voice is
490+
// synthesized) and delivers it — the one shape every policy/config refusal uses.
491+
func (r *runtime) refuse(ctx context.Context, it state.Item, msg string) {
492+
it.Voice = ""
493+
if serr := r.gw.SetReplied(ctx, it.Channel, it.MessageID, msg, ""); serr != nil {
494+
fmt.Fprintf(r.out, "gateway: recording refusal for %s: %v\n", it.Channel, serr)
495+
return
496+
}
497+
r.deliverReply(ctx, it, msg)
498+
}
499+
499500
// deliverReply sends a finished job's reply and, on success, marks the item done.
500501
// A transient send failure is retried in-process a few times; if it still fails
501502
// the item stays 'replied' and the worker retries it on a later tick and after a
@@ -509,7 +510,14 @@ func (r *runtime) deliverReply(ctx context.Context, it state.Item, reply string)
509510
if strings.TrimSpace(reply) == "" {
510511
reply = "Done."
511512
}
512-
out := channels.Outbound{Text: reply, VoicePath: r.maybeSpeak(ctx, it, reply)}
513+
out := channels.Outbound{Text: reply}
514+
// The voice rendition was synthesized once at job completion; delivery only
515+
// resolves its spool ID (missing/pruned file → text only, never an error).
516+
if it.Voice != "" {
517+
if p, err := channels.ResolveSpoolID(r.mediaDir, it.Voice); err == nil {
518+
out.VoicePath = p
519+
}
520+
}
513521
var sendErr error
514522
for attempt := 0; attempt < 3; attempt++ {
515523
if attempt > 0 {

‎internal/gateway/state/state.go‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ CREATE TABLE IF NOT EXISTS inbox (
3636
agent TEXT NOT NULL DEFAULT '', -- persona snapshot at receipt (immutable for this task)
3737
project TEXT NOT NULL DEFAULT '', -- project id snapshot at receipt (immutable for this task)
3838
attachments TEXT NOT NULL DEFAULT '', -- JSON array of media spool IDs riding this message
39+
voice TEXT NOT NULL DEFAULT '', -- spool ID of the synthesized voice reply (synthesized ONCE, at job completion)
3940
received_at TEXT NOT NULL,
4041
PRIMARY KEY (channel, message_id)
4142
);
@@ -111,6 +112,7 @@ type Item struct {
111112
Agent string // persona snapshot at receipt
112113
Project string // project id snapshot at receipt
113114
Attachments []string // media spool IDs (bare filenames; resolved only inside the spool)
115+
Voice string // spool ID of the synthesized voice reply ("" = text only)
114116
}
115117

116118
// Store is the gateway's durable state.
@@ -162,6 +164,7 @@ func Open(ctx context.Context, dir string) (*Store, error) {
162164
`ALTER TABLE inbox ADD COLUMN agent TEXT NOT NULL DEFAULT ''`,
163165
`ALTER TABLE inbox ADD COLUMN project TEXT NOT NULL DEFAULT ''`,
164166
`ALTER TABLE inbox ADD COLUMN attachments TEXT NOT NULL DEFAULT ''`,
167+
`ALTER TABLE inbox ADD COLUMN voice TEXT NOT NULL DEFAULT ''`,
165168
} {
166169
if _, err := db.ExecContext(ctx, col); err != nil && !strings.Contains(err.Error(), "duplicate column") {
167170
_ = db.Close()
@@ -258,14 +261,16 @@ func (s *Store) Pending(ctx context.Context) ([]Item, error) {
258261
return out, rows.Err()
259262
}
260263

261-
// SetReplied durably records a finished job's reply and moves the item to
262-
// 'replied'. From here the job is never re-run; only the reply's delivery is
263-
// retried, so a send failure or a crash after the job completes cannot lose the
264-
// result or repeat the work.
265-
func (s *Store) SetReplied(ctx context.Context, channel, messageID, reply string) error {
264+
// SetReplied durably records a finished job's reply — and, when one was
265+
// synthesized, the spool ID of its voice rendition — and moves the item to
266+
// 'replied'. From here the job is never re-run and the voice is never
267+
// re-synthesized; only the reply's delivery is retried, so a send failure, a
268+
// crash, or a down channel cannot lose the result, repeat the work, or bill
269+
// TTS twice.
270+
func (s *Store) SetReplied(ctx context.Context, channel, messageID, reply, voice string) error {
266271
_, err := s.db.ExecContext(ctx,
267-
`UPDATE inbox SET status = 'replied', reply = ? WHERE channel = ? AND message_id = ?`,
268-
reply, channel, messageID)
272+
`UPDATE inbox SET status = 'replied', reply = ?, voice = ? WHERE channel = ? AND message_id = ?`,
273+
reply, voice, channel, messageID)
269274
return err
270275
}
271276

@@ -274,7 +279,7 @@ func (s *Store) SetReplied(ctx context.Context, channel, messageID, reply string
274279
// and replayed after a restart.
275280
func (s *Store) PendingReplies(ctx context.Context) ([]Item, error) {
276281
rows, err := s.db.QueryContext(ctx,
277-
`SELECT channel, message_id, conversation, principal, text, trusted, reply
282+
`SELECT channel, message_id, conversation, principal, text, trusted, reply, voice
278283
FROM inbox WHERE status = 'replied' ORDER BY received_at`)
279284
if err != nil {
280285
return nil, fmt.Errorf("pending replies: %w", err)
@@ -284,7 +289,7 @@ func (s *Store) PendingReplies(ctx context.Context) ([]Item, error) {
284289
for rows.Next() {
285290
var it Item
286291
var trusted int
287-
if err := rows.Scan(&it.Channel, &it.MessageID, &it.Conversation, &it.Principal, &it.Text, &trusted, &it.Reply); err != nil {
292+
if err := rows.Scan(&it.Channel, &it.MessageID, &it.Conversation, &it.Principal, &it.Text, &trusted, &it.Reply, &it.Voice); err != nil {
288293
return nil, err
289294
}
290295
it.Trusted = trusted != 0

0 commit comments

Comments
 (0)