Resolve text approvals against the oldest pending approval - #2008
Merged
Conversation
Aaronontheweb
added
bug
Aaronontheweb
force-pushed
the
fix/approval-text-match-order
branch
from
August 19, 2026 02:52
728b8f6 to
1dc7d73
Compare
Aaronontheweb
force-pushed
the
fix/approval-text-match-order
branch
from
August 19, 2026 03:39
1dc7d73 to
728b8f6
Compare
Aaronontheweb
marked this pull request as ready for review
August 19, 2026 06:23
Aaronontheweb
force-pushed
the
fix/approval-text-match-order
branch
from
August 19, 2026 12:26
728b8f6 to
6e40b93
Compare
Aaronontheweb
changed the title
Aaronontheweb
commented
Aug 19, 2026
Aaronontheweb
left a comment
Aaronontheweb
left a comment
Collaborator
Author
There was a problem hiding this comment.
All text channels now process text-based approvals oldest-> newest by default.
Aaronontheweb
commented
Aug 19, 2026
| // millisecond suffix collided when two fires for one reminder landed in | ||
| // the same millisecond and threw InvalidActorNameException. The actor is | ||
| // single-threaded, so a plain counter is collision-free for its lifetime. | ||
| private long _executionSequence; |
Discord and Mattermost resolved the most recent pending approval. Slack resolved the earliest one. The consolidation found the difference. The maintainer decided that one order applies to every channel: the earliest pending approval wins. Changes: - PendingApprovalLookup selects the earliest match. It no longer takes a match order. The requester check keeps its structure. - ApprovalResponseFlow drops the matchOrder dependency. - The three binding actors drop the matchOrder argument. - The ApprovalMatchOrder enum is removed. No caller needs it. - A new cross-channel contract test proves that a text reply resolves the earliest of two pending approvals on Slack, Discord, and Mattermost. - The channel-binding-parity spec records the maintainer decision.
Two fires for one reminder inside the same millisecond produced the same child actor name and threw InvalidActorNameException — observed as a macOS CI failure in the retry test. The actor is single-threaded, so a monotonic counter is collision-free for its lifetime. The start time stays tracked in the active-execution table where it belongs.
Aaronontheweb
force-pushed
the
fix/approval-text-match-order
branch
from
August 19, 2026 16:41
30ff5d2 to
250afee
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
PR 7 of the code-reduction stack (on top of #2007). Implements the maintainer decision on the match-order divergence that the binding-engine consolidation (#2005) surfaced and documented: a text approval reply resolves the OLDEST pending approval (first in, first resolved), on every channel.
ApprovalMatchOrderenum and parameter are removed entirely;PendingApprovalLookup.Resolvehard-codes oldest-first with a remark recording the decision. The requester check is structurally unchanged.Text_approval_response_resolves_earliest_pending_approval: two pending approvals, first text reply resolves the earliest, second reply resolves the remaining one (also proving removal). Runs per channel; fails under the oldNewestbehavior. No existing test pinned the old behavior.Verification
Netclaw.Actors.Tests: 3,443 / 0 / 1 pre-existing skipStack
Base:
refactor/remove-vestigial(#2007). Note: the whole stack was rebased onto currentdevin the same push.