Skip to content

publish_tracked_set extends without de-duplication, admitting duplicate tracked-set members #7504

Description

@lgray

🤖 AI text below 🤖

Summary

publish_tracked_set appends to an existing tracked set with a bare .extend() and no de-duplication, so an object already in the set is added a second time when another producer in the same resolution chain harvests it again.

crates/engine/src/game/effects/mod.rs:6419-6424 — when chain_tracked_set_id is Some, the extend path runs .extend(affected_ids) against the existing member list without checking membership.

Why it is not currently observable

This is masked, not absent. At main today, a second producer cannot extend a set that a first producer already published in the same chain, so the duplicate never materializes in normal play. The defect is reachable only when that structural guarantee is removed.

That masking is strengthened by PR #7484 (mode-scoped tracked-set handoff), which adds a mode-boundary reset and publish-walk stops. To be explicit, since this was found while working on that PR: #7484 masks this defect further; it does not cause it. The bare .extend() predates it.

Reproduction (both observed under deliberately reverted/mis-keyed builds)

Both of the following were produced while probing #7484, by removing a guard and running the existing integration suite:

probe observed set contents
crossing #3 reverted (publish-walk stop removed from later_node_is_publisher_position) [[1, 2, 2]] — object 2 appended twice
mode-boundary reset mis-keyed on sub_link instead of the mode ordinal [[1, 1, 2, 2, 3, 4], []] — objects 1 and 2 each duplicated

In both cases a second producer in the same chain harvested objects already present in the set, and the extend path re-appended them.

Why it is worth fixing anyway

The set is consumed as a population — its members are iterated to apply effects and its size is read by quantity references (TrackedSetSize, FilteredTrackedSetSize, TrackedSetAggregate, and the CardTypeSetSource::TrackedSet / PlayerFilter::TrackedSetPossessor paths). A duplicated member therefore risks:

  • double application of a per-member effect, and
  • an inflated count for any "for each" / size-derived quantity.

Both are silent — a duplicate looks like a legitimately larger population. The current safety rests entirely on an invariant enforced elsewhere; a de-dup at the point of extension would make the set's own representation correct regardless of what upstream guarantees hold.

Suggested fix

De-duplicate at the extend site (preserving first-seen order, which is what CR 608.2c's "order written" consumers expect), rather than relying on producers never overlapping.

Provenance

Found while building revert probes for #7484. Related: #7500, #7501 (also filed from that work). Not fixed in #7484 — that PR's scope is the mode boundary, and fixing an unrelated pre-existing defect inside it would have widened the diff without a discriminating test in that PR's own scope.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions