Skip to content

Fix redelivery Issue #710 - #731

Open
mhelsley wants to merge 2 commits into
charmbracelet:mainfrom
mhelsley:fix-redelivery
Open

Fix redelivery Issue #710#731
mhelsley wants to merge 2 commits into
charmbracelet:mainfrom
mhelsley:fix-redelivery

Conversation

@mhelsley

@mhelsley mhelsley commented Feb 7, 2026

Copy link
Copy Markdown

I believe these two commits are ready for review. The first creates a test which reproduces the reported issue and the second fixes it.

If I understand the below correctly, since this isn't a new feature it is not necessary to contact a maintainer before submitting this PR.

  • I have read CONTRIBUTING.md.
  • I have created a discussion that was approved by a maintainer (for new features).

@mhelsley
mhelsley requested a review from a team as a code owner February 7, 2026 23:26
@mhelsley
mhelsley requested review from aymanbagabas and meowgorithm and removed request for a team February 7, 2026 23:26
@mhelsley

mhelsley commented Feb 8, 2026

Copy link
Copy Markdown
Author

This is an alternative to PR #726 . My apologies, I forgot to look for pre-existing PRs. This one has a test case in the first commit which may still be useful to merge even if the other PR is merged.

Matt Helsley added 2 commits February 11, 2026 11:51
Reproduces Issue charmbracelet#710: Duplicate message forwarding in Group.Update

Count the number of times a sent message is delivered to
the same model instance and also count deliveries to a
selected field using a custom field type.

The assumption is a given message instance will not be delivered
twice to the same selected field.

This testcase shows that non-KeyMsg messages are delivered
twice to a group's selected field:

❯ go test
--- FAIL: TestCountMsgRedelivery (0.00s)
    group_test.go:207: INFO: Estimated 4 or more other type of tea.Msg deliveries to top level model got 5
    group_test.go:250: Expected 1 testMsg deliveries to counter model got 2

When a subsequent fix is applied the testcase succeeds
and produces no output -- it has verified that exactly one testMsg
gets delivered to the selected field.

Includes a sanity check using the Select Field in place of
the custom Field that only counts the messages delivered at
the top level of the model. It verifies that the messages
sent by the testcase to the program are sufficient to manipulate
a standard Field to produce a form result. This way if the
test methodology ever becomes out-of-date it will be clear how to
update it.
Fixes Issue charmbracelet#710 Duplicate message forwarding in Group.Update

Non-KeyMsg are passed into Update twice for the selected
field. It happens in the default block first then,
after exiting the switch, inside the index-guarded
block.

Now every field still receives non-Key messages but only
the selected field receives KeyMsg

Refs: Issue charmbracelet#710
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant