Merge pull request #29 from cuiko/fix/msg-delete-affected-count

fix(msg): Report the count Telegram actually deleted, not requested

Author: cuiko tang

internal/action/message/ops.go

@@ -433,8 +433,8 @@ type DeleteResult struct {
 	Count int
 }
 
-// DeleteFunc deletes messages.
-type DeleteFunc func(context.Context, DeleteQuery) error
+// DeleteFunc deletes messages and returns how many Telegram actually affected.
+type DeleteFunc func(context.Context, DeleteQuery) (int, error)
 
 // Delete validates, confirms, and dispatches a delete request.
 func Delete(ctx context.Context, req DeleteRequest, do DeleteFunc) (DeleteResult, error) {
@@ -452,14 +452,18 @@ func Delete(ctx context.Context, req DeleteRequest, do DeleteFunc) (DeleteResult
 	if err := ui.ConfirmDestructive(req.Prompter, fmt.Sprintf("%s %d message(s) in %s?", verb, len(query.IDs), query.Ref.String()), req.Yes); err != nil {
 		return DeleteResult{}, err
 	}
-	if err := do(ctx, query); err != nil {
+	affected, err := do(ctx, query)
+	if err != nil {
 		return DeleteResult{}, err
 	}
 	outVerb := "deleted"
 	if query.Revoke {
 		outVerb = "revoked"
 	}
-	return DeleteResult{Verb: outVerb, Count: len(query.IDs)}, nil
+	// Report the count Telegram actually affected, not the number requested:
+	// a self-side delete of another user's message in a supergroup is a no-op
+	// the server reports as 0.
+	return DeleteResult{Verb: outVerb, Count: affected}, nil
 }
 
 // NormalizeDelete parses delete refs.

internal/action/message/ops_test.go

@@ -250,24 +250,26 @@ func TestDelete_ConfirmsBeforeDispatch(t *testing.T) {
 	result, err := actionmessage.Delete(context.Background(), actionmessage.DeleteRequest{
 		RawMessageRefs: []string{"@chat:1", "@chat:2"},
 		Prompter:       stubPrompter{ok: true},
-	}, func(_ context.Context, q actionmessage.DeleteQuery) error {
+	}, func(_ context.Context, q actionmessage.DeleteQuery) (int, error) {
 		called = true
 		require.Equal(t, []int{1, 2}, q.IDs)
-		return nil
+		// Server reports only one affected though two IDs were requested; the
+		// result count must come from this return value, not len(q.IDs).
+		return 1, nil
 	})
 	require.NoError(t, err)
 	require.True(t, called)
-	require.Equal(t, actionmessage.DeleteResult{Verb: "deleted", Count: 2}, result)
+	require.Equal(t, actionmessage.DeleteResult{Verb: "deleted", Count: 1}, result)
 }
 
 func TestDelete_DeclineSkipsDispatch(t *testing.T) {
 	called := false
 	_, err := actionmessage.Delete(context.Background(), actionmessage.DeleteRequest{
 		RawMessageRefs: []string{"@chat:1"},
 		Prompter:       stubPrompter{ok: false},
-	}, func(context.Context, actionmessage.DeleteQuery) error {
+	}, func(context.Context, actionmessage.DeleteQuery) (int, error) {
 		called = true
-		return nil
+		return 0, nil
 	})
 	require.ErrorIs(t, err, command.ErrNotConfirmed)
 	require.False(t, called)

internal/cli/msg/delete/delete.go

@@ -3,6 +3,7 @@ package deletecmd
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 
 	"github.com/gotd/td/telegram/peers"
@@ -72,19 +73,37 @@ func Run(ctx context.Context, opts *Options) error {
 
 // newDelete returns the production Delete closure that calls the Telegram API.
 func newDelete(f *runtime.Invocation) actionmessage.DeleteFunc {
-	return func(ctx context.Context, q actionmessage.DeleteQuery) error {
+	return func(ctx context.Context, q actionmessage.DeleteQuery) (int, error) {
 		acct, err := f.Account("")
 		if err != nil {
-			return err
+			return 0, err
 		}
 		if cl, _ := runtime.MaybeDialDaemon(ctx, f, acct); cl != nil {
 			defer func() { _ = cl.Close() }()
-			_, err := cl.Call(ctx, "msg.delete", q)
-			return err
+			raw, err := cl.Call(ctx, "msg.delete", q)
+			if err != nil {
+				return 0, err
+			}
+			return decodeDeleteCount(raw, len(q.IDs)), nil
 		}
-		return f.WithPeers(ctx, acct, runtime.ClientOptsFrom(f, acct),
+		var affected int
+		err = f.WithPeers(ctx, acct, runtime.ClientOptsFrom(f, acct),
 			func(ctx context.Context, api *tg.Client, _ *peers.Manager, res *peer.Resolver) error {
-				return telegram.DeleteMessages(ctx, api, res, q)
+				affected, err = telegram.DeleteMessages(ctx, api, res, q)
+				return err
 			})
+		return affected, err
+	}
+}
+
+// decodeDeleteCount reads the affected-message count from a daemon msg.delete
+// response. A current daemon returns the integer count; an older one returns a
+// bare "true" ack that won't decode, in which case we fall back to the
+// requested count so delete still works across a version skew.
+func decodeDeleteCount(raw json.RawMessage, requested int) int {
+	var affected int
+	if err := json.Unmarshal(raw, &affected); err != nil {
+		return requested
 	}
+	return affected
 }

internal/cli/msg/delete/delete_test.go

@@ -30,7 +30,7 @@ func TestRun_NoYesNoPrompter_Declined(t *testing.T) {
 	f.Prompter = stubPrompter{ok: false}
 	opts := &del.Options{
 		RawMessageRefs: []string{"@a:1"}, Prompter: f.Prompter, IOStreams: ios,
-		Delete: func(_ context.Context, _ actionmessage.DeleteQuery) error { return nil },
+		Delete: func(_ context.Context, _ actionmessage.DeleteQuery) (int, error) { return 0, nil },
 	}
 	err := del.Run(context.Background(), opts)
 	require.ErrorIs(t, err, command.ErrNotConfirmed)
@@ -42,11 +42,13 @@ func TestRun_YesSkipsPromptCallsDelete(t *testing.T) {
 	called := false
 	opts := &del.Options{
 		RawMessageRefs: []string{"@a:1", "@a:2"}, Yes: true, Prompter: f.Prompter, IOStreams: ios,
-		Delete: func(_ context.Context, _ actionmessage.DeleteQuery) error { called = true; return nil },
+		// Two refs requested but Telegram reports only one affected — the
+		// output must reflect the affected count, not len(refs).
+		Delete: func(_ context.Context, _ actionmessage.DeleteQuery) (int, error) { called = true; return 1, nil },
 	}
 	require.NoError(t, del.Run(context.Background(), opts))
 	require.True(t, called)
-	require.Contains(t, stdout.String(), "deleted")
+	require.Contains(t, stdout.String(), "deleted\t1")
 }
 
 func TestRun_PromptAcceptedCallsDelete(t *testing.T) {
@@ -56,7 +58,7 @@ func TestRun_PromptAcceptedCallsDelete(t *testing.T) {
 	called := false
 	opts := &del.Options{
 		RawMessageRefs: []string{"@a:5"}, Prompter: f.Prompter, IOStreams: ios,
-		Delete: func(_ context.Context, _ actionmessage.DeleteQuery) error { called = true; return nil },
+		Delete: func(_ context.Context, _ actionmessage.DeleteQuery) (int, error) { called = true; return 1, nil },
 	}
 	require.NoError(t, del.Run(context.Background(), opts))
 	require.True(t, called)
@@ -76,13 +78,21 @@ func TestRun_RevokeOutputsRevoked(t *testing.T) {
 	f := runtime.NewTestInvocation(t)
 	opts := &del.Options{
 		RawMessageRefs: []string{"@a:1"}, Revoke: true, Yes: true, Prompter: f.Prompter, IOStreams: ios,
-		Delete: func(_ context.Context, _ actionmessage.DeleteQuery) error { return nil },
+		Delete: func(_ context.Context, _ actionmessage.DeleteQuery) (int, error) { return 1, nil },
 	}
 	require.NoError(t, del.Run(context.Background(), opts))
 	require.Contains(t, stdout.String(), "revoked")
 	require.NotContains(t, stdout.String(), "deleted")
 }
 
+func TestDecodeDeleteCount(t *testing.T) {
+	// Current daemon: integer affected count is used as-is.
+	require.Equal(t, 3, del.DecodeDeleteCountForTest([]byte("3"), 5))
+	require.Equal(t, 0, del.DecodeDeleteCountForTest([]byte("0"), 5))
+	// Older daemon: bare "true" ack doesn't decode -> fall back to requested.
+	require.Equal(t, 5, del.DecodeDeleteCountForTest([]byte("true"), 5))
+}
+
 type stubPrompter struct{ ok bool }
 
 func (s stubPrompter) Confirm(string, bool) (bool, error)   { return s.ok, nil }

internal/cli/msg/delete/export_test.go

@@ -0,0 +1,9 @@
+package deletecmd
+
+import "encoding/json"
+
+// DecodeDeleteCountForTest exposes decodeDeleteCount to the external test
+// package.
+func DecodeDeleteCountForTest(raw json.RawMessage, requested int) int {
+	return decodeDeleteCount(raw, requested)
+}

internal/daemon/handlers.go

@@ -155,10 +155,11 @@ func registerHandlers(
 		if err := json.Unmarshal(params, &q); err != nil { //nolint:musttag
 			return nil, fmt.Errorf("invalid msg.delete params: %w", err)
 		}
-		if err := telegram.DeleteMessages(ctx, api, res, q); err != nil {
+		affected, err := telegram.DeleteMessages(ctx, api, res, q)
+		if err != nil {
 			return nil, err
 		}
-		return json.RawMessage("true"), nil
+		return json.Marshal(affected)
 	})
 
 	srv.Register("msg.pin", func(ctx context.Context, params json.RawMessage) (json.RawMessage, error) {

internal/telegram/message_ops.go

@@ -292,20 +292,29 @@ func ForwardMessages(ctx context.Context, api *tg.Client, resolver *peer.Resolve
 	return rows[0], nil
 }
 
-// DeleteMessages performs the Telegram RPC for `tg msg delete`.
-func DeleteMessages(ctx context.Context, api *tg.Client, resolver *peer.Resolver, q actionmessage.DeleteQuery) error {
+// DeleteMessages performs the Telegram RPC for `tg msg delete`. It returns the
+// number of messages Telegram actually affected (PtsCount), which can be less
+// than len(q.IDs) — e.g. deleting someone else's message self-side in a
+// supergroup is a no-op the server reports as 0.
+func DeleteMessages(ctx context.Context, api *tg.Client, resolver *peer.Resolver, q actionmessage.DeleteQuery) (int, error) {
 	sender := gotdmessage.NewSender(api)
 	if !q.Revoke {
-		_, err := sender.Delete().Messages(ctx, q.IDs...)
-		return err
+		affected, err := sender.Delete().Messages(ctx, q.IDs...)
+		if err != nil {
+			return 0, err
+		}
+		return affected.PtsCount, nil
 	}
 
 	resolved, err := resolver.Resolve(ctx, q.Ref)
 	if err != nil {
-		return err
+		return 0, err
 	}
-	_, err = sender.To(resolved.InputPeer).Revoke().Messages(ctx, q.IDs...)
-	return err
+	affected, err := sender.To(resolved.InputPeer).Revoke().Messages(ctx, q.IDs...)
+	if err != nil {
+		return 0, err
+	}
+	return affected.PtsCount, nil
 }
 
 // PinMessage performs the Telegram RPC for `tg msg pin` and `tg msg unpin`.