Repository navigation
feat: add optional final native-frame admission hook #52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| # Optional native send admission (dark integration seam) | ||
|
|
||
| `SendRequestExtra.NativeSend` is process-local and nil by default. The existing | ||
| send path is unchanged when nil. Initial support is one ordinary text DM with | ||
| an explicitly preallocated message ID; group, peer, bot, media and extra-node | ||
| paths are refused before preparation. This is not a renderer or a grant. | ||
|
|
||
| `Admit(ctx, id, write)` runs after native preparation and after the Noise socket | ||
| write lock. It must synchronously invoke `write` at most once inside the final | ||
| local/authority fence. Returning without invocation refuses the send. A retained | ||
| callback cannot invoke after Admit returns. A refusal consumes no Noise nonce | ||
| or first-frame header. `Entered()` is a non-blocking notification immediately | ||
| before the actual WebSocket write call; it is evidence of entry, not provider | ||
| acceptance. It cannot prove whether bytes reached a provider. | ||
|
|
||
| The hook returns before the provider ACK wait. Callers must not hold Campaign | ||
| locks through that wait. Persist an invocation CAS marker and preallocated ID | ||
| before writing; an existing marker with no durable outcome is unknown and must | ||
| never be reinvoked. No goroutine-start or AMQP-publish callback counts as entry. | ||
| A frame-write failure or post-entry authority loss is not proof of non-delivery. | ||
|
|
||
| Guarded sends do not populate HyperMeow's retry content cache/store and do not | ||
| use the disconnect frame resend path. Integrators must also keep their | ||
| `GetMessageForRetry` fallback from returning guarded Campaign content and must | ||
| not implement an automatic retry around this API. Durable retry/receipt policy | ||
| and the regional invocation/result outbox belong to the owning application. | ||
| No Campaign consumer, positive authority adapter, database schema or runtime | ||
| mount is supplied here. This seam alone does not establish native acceptance. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| package whatsmeow | ||
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
|
|
||
| "google.golang.org/protobuf/reflect/protoreflect" | ||
|
|
||
| waBinary "github.com/polymorfa/hypermeow/binary" | ||
| "github.com/polymorfa/hypermeow/proto/waE2E" | ||
| "github.com/polymorfa/hypermeow/types" | ||
| ) | ||
|
|
||
| var ErrNativeSendUnsupported = errors.New("native admission requires a preallocated ordinary text DM") | ||
|
|
||
| // NativeSendHooks are process-local. Admit must synchronously invoke write at | ||
| // most once under the caller's final local/authority fence. Entered is a short | ||
| // notification immediately before the actual socket write; it must not block. | ||
| // The hooks finish before SendMessage waits for the provider ACK. | ||
| type NativeSendHooks struct { | ||
| Admit func(context.Context, types.MessageID, func() error) error | ||
| Entered func() | ||
| } | ||
|
|
||
| func (cli *Client) sendNativeMessageNode(ctx context.Context, node waBinary.Node, id types.MessageID, hooks *NativeSendHooks) ([]byte, error) { | ||
| if hooks == nil { | ||
| return cli.sendNodeAndGetData(ctx, node) | ||
| } | ||
| if hooks.Admit == nil || hooks.Entered == nil || id == "" { | ||
| return nil, ErrNativeSendUnsupported | ||
| } | ||
| cli.socketLock.RLock() | ||
| sock := cli.socket | ||
| cli.socketLock.RUnlock() | ||
| // A shadow relay is a separate transport, not proof of native socket entry. | ||
| if sock == nil { | ||
| return nil, ErrNotConnected | ||
| } | ||
| payload, err := waBinary.Marshal(node) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| err = sock.SendFrameAdmitted(ctx, payload, func(ctx context.Context, write func() error) error { | ||
| return hooks.Admit(ctx, id, write) | ||
| }, hooks.Entered) | ||
| return payload, err | ||
| } | ||
|
|
||
| // Restrict the initial hook to one ordinary text payload. Unknown fields and | ||
| // sibling message variants must not smuggle additional native effects. | ||
| func validNativeText(message *waE2E.Message) bool { | ||
| if message == nil || message.Conversation == nil || len(message.ProtoReflect().GetUnknown()) != 0 { | ||
| return false | ||
| } | ||
| valid := true | ||
| message.ProtoReflect().Range(func(field protoreflect.FieldDescriptor, _ protoreflect.Value) bool { | ||
| if field.Name() != "conversation" { | ||
| valid = false | ||
| } | ||
| return valid | ||
| }) | ||
| return valid | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| package whatsmeow | ||
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
| "testing" | ||
|
|
||
| "google.golang.org/protobuf/proto" | ||
|
|
||
| "github.com/polymorfa/hypermeow/proto/waE2E" | ||
| "github.com/polymorfa/hypermeow/types" | ||
| ) | ||
|
|
||
| func TestNativeSendRejectsUnsupportedBeforePreparation(t *testing.T) { | ||
| cli := &Client{} | ||
| to := types.NewJID("15550000000", types.DefaultUserServer) | ||
| hooks := &NativeSendHooks{Admit: func(context.Context, types.MessageID, func() error) error { | ||
| t.Fatal("unexpected admission") | ||
| return nil | ||
| }, Entered: func() { t.Fatal("unexpected entry") }} | ||
| for _, tc := range []struct { | ||
| name string | ||
| message *waE2E.Message | ||
| extra SendRequestExtra | ||
| }{ | ||
| {"missing ID", &waE2E.Message{Conversation: proto.String("text")}, SendRequestExtra{NativeSend: hooks}}, | ||
| {"media", &waE2E.Message{ImageMessage: &waE2E.ImageMessage{}}, SendRequestExtra{ID: "original", NativeSend: hooks}}, | ||
| {"sibling", &waE2E.Message{Conversation: proto.String("text"), ImageMessage: &waE2E.ImageMessage{}}, SendRequestExtra{ID: "original", NativeSend: hooks}}, | ||
| {"peer", &waE2E.Message{Conversation: proto.String("text")}, SendRequestExtra{ID: "original", NativeSend: hooks, Peer: true}}, | ||
| {"missing callback", &waE2E.Message{Conversation: proto.String("text")}, SendRequestExtra{ID: "original", NativeSend: &NativeSendHooks{}}}, | ||
| } { | ||
| t.Run(tc.name, func(t *testing.T) { | ||
| _, err := cli.SendMessage(t.Context(), to, tc.message, tc.extra) | ||
| if !errors.Is(err, ErrNativeSendUnsupported) { | ||
| t.Fatalf("wrong refusal: %v", err) | ||
| } | ||
| }) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,50 @@ | ||||||
| package socket | ||||||
|
|
||||||
| import ( | ||||||
| "context" | ||||||
| "errors" | ||||||
| "sync" | ||||||
| ) | ||||||
|
|
||||||
| var ErrNativeAdmissionClosed = errors.New("native send admission closed") | ||||||
|
|
||||||
| // SendFrameAdmitted joins a process-local admission to an actual frame write. | ||||||
| // It runs after the Noise write lock, never around goroutine scheduling or ACK | ||||||
| // waiting. A refusal consumes neither the Noise nonce nor the first-frame header. | ||||||
| func (ns *NoiseSocket) SendFrameAdmitted(ctx context.Context, plaintext []byte, admit func(context.Context, func() error) error, entered func()) error { | ||||||
| if admit == nil || entered == nil { | ||||||
| return ErrNativeAdmissionClosed | ||||||
| } | ||||||
| ns.writeLock.Lock() | ||||||
| defer ns.writeLock.Unlock() | ||||||
| if err := ctx.Err(); err != nil { | ||||||
| return err | ||||||
| } | ||||||
| ciphertext := ns.writeKey.Seal(nil, generateIV(ns.writeCounter), plaintext, nil) | ||||||
| var mu sync.Mutex | ||||||
| active, invoked := true, false | ||||||
| var writeErr error | ||||||
| err := admit(ctx, func() error { | ||||||
| mu.Lock() | ||||||
| defer mu.Unlock() | ||||||
| if !active || invoked { | ||||||
| return ErrNativeAdmissionClosed | ||||||
| } | ||||||
| if err := ctx.Err(); err != nil { | ||||||
| return err | ||||||
| } | ||||||
| invoked = true | ||||||
| ns.writeCounter++ | ||||||
| writeErr = ns.fs.sendFrame(ciphertext, entered, ctx) | ||||||
|
Comment on lines
+37
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When an admitted payload's ciphertext is at least Useful? React with 👍 / 👎. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win Do not use the caller's Line 38 passes the per-send Line 20 and line 33 already check 🐛 Proposed fix--- "a/socket/native_admission.go"
+++ "b/socket/native_admission.go"
@@ -35,7 +35,7 @@
}
invoked = true
ns.writeCounter++
- writeErr = ns.fs.sendFrame(ciphertext, entered, ctx)
+ writeErr = ns.fs.sendFrame(ciphertext, entered, nil)
return writeErr
})
mu.Lock()If the per-send context is required, keep it out of 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| return writeErr | ||||||
| }) | ||||||
| mu.Lock() | ||||||
| active = false | ||||||
|
Comment on lines
+41
to
+42
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If Useful? React with 👍 / 👎. |
||||||
| didInvoke := invoked | ||||||
| result := writeErr | ||||||
| mu.Unlock() | ||||||
| if !didInvoke && err == nil { | ||||||
| return ErrNativeAdmissionClosed | ||||||
| } | ||||||
| return errors.Join(err, result) | ||||||
| } | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: polymorfa/hypermeow
Length of output: 4800
🏁 Script executed:
Repository: polymorfa/hypermeow
Length of output: 43113
🏁 Script executed:
Repository: polymorfa/hypermeow
Length of output: 4811
Keep
Admitshort while holdingNoiseSocket.writeLock.SendFrameAdmittedholds the write lock for the full synchronousAdmitcall. The current “before writing” guidance can permit a durable database or network operation insideAdmit, which can delay other serialized outbound writes.Clarify that
Admitmust be short and bounded, and must not wait on unbounded I/O. Do not require an in-memory-only fence or require every durable CAS to occur beforeSendMessage; the final authority fence and CAS implementation remain integration-owned.Suggested documentation change
📝 Committable suggestion
🤖 Prompt for AI Agents