Skip to content

fix(automation): fire tag_added only when the tag is actually added (#554) #556

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.

Sign up for GitHub

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

Merged
pikann merged 2 commits into from
Oct 8, 2026
Merged

fix(automation): fire tag_added only when the tag is actually added (#554) #556

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e92af8b
fix(automation): fire tag_added only when the tag is actually added (…
laithalenooz Oct 7, 2026
2cc35a4
Merge remote-tracking branch 'upstream/master' into fix/tag-added-fir…
laithalenooz Oct 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 39 additions & 8 deletions services/api/internal/worker/automation_consumer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,7 @@ func (c *AutomationConsumer) handle(msg redis.XMessage) {
}

var candidates []automationdom.TriggerType
var added []string
checkPredecessors := false

switch p.ActivityType {
Expand All @@ -499,7 +500,11 @@ func (c *AutomationConsumer) handle(msg redis.XMessage) {
case "importance":
candidates = append(candidates, automationdom.TriggerPriorityChanged)
case "tags":
candidates = append(candidates, automationdom.TriggerTagAdded)
// Only an actual addition is a tag_added event: removals and
// edits that keep an existing tag must not re-fire it (#554).
if added = addedTags(change); len(added) > 0 {
candidates = append(candidates, automationdom.TriggerTagAdded)
}
}
}
}
Expand All @@ -510,7 +515,7 @@ func (c *AutomationConsumer) handle(msg redis.XMessage) {
return
}

if err := c.processEvent(ctx, projectID, taskID, candidates, checkPredecessors); err != nil {
if err := c.processEvent(ctx, projectID, taskID, candidates, checkPredecessors, added); err != nil {
c.log.Error("automation consumer: failed to process event", "id", msg.ID, "task_id", taskID, "err", err)
// Do not ack — retried via processPending on next restart.
return
Expand Down Expand Up @@ -760,7 +765,7 @@ func (c *AutomationConsumer) handleSprintActivity(msg redis.XMessage) {
// automation are logged and skipped (best-effort) rather than aborting the
// whole batch. An error is returned only when the foundational task lookup
// fails, so the message is retried.
func (c *AutomationConsumer) processEvent(ctx context.Context, projectID, taskID uuid.UUID, candidates []automationdom.TriggerType, checkPredecessors bool) error {
func (c *AutomationConsumer) processEvent(ctx context.Context, projectID, taskID uuid.UUID, candidates []automationdom.TriggerType, checkPredecessors bool, added []string) error {
task, err := c.taskRepo.FindTaskByID(ctx, taskID)
if err != nil {
return fmt.Errorf("find task: %w", err)
Expand All @@ -773,7 +778,7 @@ func (c *AutomationConsumer) processEvent(ctx context.Context, projectID, taskID
continue
}
for _, node := range nodes {
if !c.triggerMatches(node, task) {
if !c.triggerMatches(node, task, added) {
continue
}
if err := c.executeRun(ctx, projectID, node, task); err != nil {
Expand Down Expand Up @@ -822,8 +827,9 @@ func (c *AutomationConsumer) processEvent(ctx context.Context, projectID, taskID

// triggerMatches checks a trigger node's config filter against the task's
// current authoritative state (not the raw activity payload — the payload
// only carries resolved names/diffs for some fields).
func (c *AutomationConsumer) triggerMatches(node *automationdom.Node, task *taskdom.Task) bool {
// only carries resolved names/diffs for some fields). added lists the tags the
// triggering update introduced; tag_added matches only those.
func (c *AutomationConsumer) triggerMatches(node *automationdom.Node, task *taskdom.Task, added []string) bool {
var cfg automationdom.TriggerConfig
if len(node.Config) > 0 {
_ = json.Unmarshal(node.Config, &cfg)
Expand All @@ -835,10 +841,12 @@ func (c *AutomationConsumer) triggerMatches(node *automationdom.Node, task *task
}
return task.StatusID != nil && *task.StatusID == *cfg.StatusID
case automationdom.TriggerTagAdded:
// added holds the tags this update introduced; the task's current tags
// confirm the tag is still there by the time the event is processed.
if cfg.Tag == "" {
return true
return len(added) > 0
}
return slices.Contains(task.Tags, cfg.Tag)
return slices.Contains(added, cfg.Tag) && slices.Contains(task.Tags, cfg.Tag)
case automationdom.TriggerTaskCreated, automationdom.TriggerAssigneeChanged, automationdom.TriggerPriorityChanged:
return true
default:
Expand Down Expand Up @@ -2626,3 +2634,26 @@ func (c *AutomationConsumer) recordAppliedActivity(ctx context.Context, projectI
Content: content,
})
}

// addedTags returns the tags present in change.New but not in change.Old, in
// the order they appear in New. Old and New arrive JSON-decoded ([]any), as
// recorded by the task handler and the autofill consumer.
func addedTags(change taskdom.FieldChange) []string {
before := map[string]bool{}
if old, ok := change.Old.([]any); ok {
for _, v := range old {
if s, ok := v.(string); ok {
before[s] = true
}
}
}
var added []string
if next, ok := change.New.([]any); ok {
for _, v := range next {
if s, ok := v.(string); ok && !before[s] {
added = append(added, s)
}
}
}
return added
}
55 changes: 48 additions & 7 deletions services/api/internal/worker/automation_consumer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func TestTriggerMatches_StatusChanged_AnyStatusWhenConfigEmpty(t *testing.T) {
statusID := uuid.New()
node := &automationdom.Node{Kind: automationdom.KindTrigger, Type: string(automationdom.TriggerStatusChanged), Config: json.RawMessage(`{}`)}
task := &taskdom.Task{StatusID: &statusID}
if !c.triggerMatches(node, task) {
if !c.triggerMatches(node, task, nil) {
t.Fatal("expected status_changed trigger with no status_id filter to match any status")
}
}
Expand All @@ -60,10 +60,10 @@ func TestTriggerMatches_StatusChanged_FiltersToConfiguredStatus(t *testing.T) {
cfg, _ := json.Marshal(automationdom.TriggerConfig{StatusID: &wantStatus})
node := &automationdom.Node{Kind: automationdom.KindTrigger, Type: string(automationdom.TriggerStatusChanged), Config: cfg}

if !c.triggerMatches(node, &taskdom.Task{StatusID: &wantStatus}) {
if !c.triggerMatches(node, &taskdom.Task{StatusID: &wantStatus}, nil) {
t.Fatal("expected match when task's status equals the configured status")
}
if c.triggerMatches(node, &taskdom.Task{StatusID: &otherStatus}) {
if c.triggerMatches(node, &taskdom.Task{StatusID: &otherStatus}, nil) {
t.Fatal("expected no match when task's status differs from the configured status")
}
}
Expand All @@ -73,19 +73,60 @@ func TestTriggerMatches_TagAdded_FiltersToConfiguredTag(t *testing.T) {
cfg, _ := json.Marshal(automationdom.TriggerConfig{Tag: "urgent"})
node := &automationdom.Node{Kind: automationdom.KindTrigger, Type: string(automationdom.TriggerTagAdded), Config: cfg}

if !c.triggerMatches(node, &taskdom.Task{Tags: []string{"urgent", "bug"}}) {
t.Fatal("expected match when the configured tag is present")
if !c.triggerMatches(node, &taskdom.Task{Tags: []string{"urgent", "bug"}}, []string{"urgent"}) {
t.Fatal("expected match when the configured tag was added")
}
if c.triggerMatches(node, &taskdom.Task{Tags: []string{"bug"}}) {
if c.triggerMatches(node, &taskdom.Task{Tags: []string{"bug"}}, []string{"bug"}) {
t.Fatal("expected no match when the configured tag is absent")
}
}

// Regression for #554: tag_added used to match whenever the configured tag was
// merely present, so any later tag edit on the task re-fired it.
func TestTriggerMatches_TagAdded_OnlyWhenThisUpdateAddedTheTag(t *testing.T) {
c := newTestConsumer(nil)
cfg, _ := json.Marshal(automationdom.TriggerConfig{Tag: "spec-ok"})
node := &automationdom.Node{Kind: automationdom.KindTrigger, Type: string(automationdom.TriggerTagAdded), Config: cfg}
task := &taskdom.Task{Tags: []string{"spec-ok", "security-ok"}}

if c.triggerMatches(node, task, []string{"security-ok"}) {
t.Fatal("expected no match when an unrelated tag was added and the configured tag was already present")
}
if !c.triggerMatches(node, task, []string{"spec-ok"}) {
t.Fatal("expected a match when this update added the configured tag")
}
}

func TestTriggerMatches_TagAdded_EmptyConfigNeedsAnAddition(t *testing.T) {
c := newTestConsumer(nil)
node := &automationdom.Node{Kind: automationdom.KindTrigger, Type: string(automationdom.TriggerTagAdded), Config: json.RawMessage(`{}`)}
task := &taskdom.Task{Tags: []string{"bug"}}

if c.triggerMatches(node, task, nil) {
t.Fatal("expected no match when the update only removed tags")
}
if !c.triggerMatches(node, task, []string{"bug"}) {
t.Fatal("expected a match when the update added any tag")
}
}

func TestAddedTags_ComparesOldAndNewAfterAJSONRoundTrip(t *testing.T) {
raw, _ := json.Marshal(taskdom.FieldChange{Field: "tags", Old: []string{"a", "b"}, New: []string{"b", "c", "d"}})
var change taskdom.FieldChange
if err := json.Unmarshal(raw, &change); err != nil {
t.Fatal(err)
}
got := addedTags(change)
if strings.Join(got, ",") != "c,d" {
t.Fatalf("expected [c d], got %v", got)
}
}

func TestTriggerMatches_UnconditionalTypesAlwaysMatch(t *testing.T) {
c := newTestConsumer(nil)
for _, tt := range []automationdom.TriggerType{automationdom.TriggerTaskCreated, automationdom.TriggerAssigneeChanged, automationdom.TriggerPriorityChanged} {
node := &automationdom.Node{Kind: automationdom.KindTrigger, Type: string(tt), Config: json.RawMessage(`{}`)}
if !c.triggerMatches(node, &taskdom.Task{}) {
if !c.triggerMatches(node, &taskdom.Task{}, nil) {
t.Errorf("expected trigger type %q to always match", tt)
}
}
Expand Down
Loading