From 82adc76c064bc2c95b0d15d95fa445301fb29a84 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Thu, 16 Jul 2026 04:18:53 -0400 Subject: [PATCH] fix: run tag filtering after --start-at-task gate Tag filtering previously ran before the `--start-at-task` gate, so a `--start-at-task` target excluded by `--tags`/`--skip-tags` was removed before the gate could resume, leaving every surviving task skipped as `before --start-at-task` and the run exiting 0 having changed nothing. Tags now narrow the survivors after the resume point is selected, matching the filter order documented in the flag help and command reference. --- commands/apply.go | 29 +++++++++- commands/list_tasks_test.go | 109 ++++++++++++++++++++++++++++++++++++ docs/command-reference.md | 4 +- docs/task-envelope.md | 5 ++ tasks/envelope.go | 31 +++++++--- tasks/envelope_test.go | 35 ++++++++++++ 6 files changed, 202 insertions(+), 11 deletions(-) diff --git a/commands/apply.go b/commands/apply.go index 570bfeb..c44f4fb 100644 --- a/commands/apply.go +++ b/commands/apply.go @@ -304,7 +304,14 @@ playLoop: } failed := false - for _, name := range tasks.FilterByTags(play.Tasks, c.tags, c.skipTags) { + // Iterate the full task list in source order. Tag filtering + // (--tags/--skip-tags) is applied inside executeTask, *after* + // the --start-at-task gate, so the documented filter order holds + // (start-at-task selects the resume point first, then tags + // narrow). Pre-filtering here would drop the --start-at-task + // target before the gate could flip `started`, silently no-oping + // the run. + for _, name := range play.Tasks.Keys() { env := play.Tasks.GetEnvelope(name) outcome := c.executeTask(env, name, ac, nil, "") if outcome.abort { @@ -374,7 +381,19 @@ type applyTaskOutcome struct { // group whose own name does not match but a descendant does is // entered so the recursive executeTask call lands on the matched // child. +// +// Tag filtering (--tags/--skip-tags) runs *after* the gate so the +// documented order holds - start-at-task selects the resume point, +// then tags narrow the survivors. It applies only to top-level +// entries (phase == ""); group children are never tag-filtered. A +// resume-point group (one entered to reach a nested target) bypasses +// the tag filter so the named child stays reachable. func (c *ApplyCommand) executeTask(env *tasks.TaskEnvelope, name string, ac *applyContext, failedTask interface{}, phase string) applyTaskOutcome { + // resumeGroup marks a group we descend into to reach a nested + // --start-at-task target; such a group skips tag filtering below so + // the descent can land on the matched child regardless of the + // group's own tags. + resumeGroup := false if !*ac.started && ac.startAtTask != "" { switch { case name == ac.startAtTask: @@ -383,6 +402,7 @@ func (c *ApplyCommand) executeTask(env *tasks.TaskEnvelope, name string, ac *app // Don't skip; descend into the group so the matching // child runs. The synthesized group state will reflect // only the executed children, not the skipped ones. + resumeGroup = true default: ac.counts.Tasks++ ac.counts.Skipped++ @@ -398,6 +418,13 @@ func (c *ApplyCommand) executeTask(env *tasks.TaskEnvelope, name string, ac *app return applyTaskOutcome{skipped: true} } } + // Drop a top-level entry the tag filter excludes. This runs after the + // gate above so the --start-at-task target still flips `started` even + // when its own tags exclude it. Excluded tasks produce no event and no + // counts, matching the prior FilterByTags pre-pass. + if phase == "" && !resumeGroup && !tasks.EnvelopePassesTags(env, c.tags, c.skipTags) { + return applyTaskOutcome{skipped: true} + } if env.IsGroup() { return c.executeGroup(env, name, ac, failedTask, phase) } diff --git a/commands/list_tasks_test.go b/commands/list_tasks_test.go index 77c11c3..84028cc 100644 --- a/commands/list_tasks_test.go +++ b/commands/list_tasks_test.go @@ -305,3 +305,112 @@ func TestApplyStartAtTaskInsideBlock(t *testing.T) { t.Errorf("block-c should run after the match; got:\n%s", stdout) } } + +// TestApplyStartAtTaskThenTags is the #315 regression: tag filtering +// must run *after* the --start-at-task gate. The target task itself is +// excluded by --tags, so under the old (tags-first) order the run +// silently no-oped. The documented order selects the resume point +// first, then narrows by tags, so a later tag-matching task runs. +func TestApplyStartAtTaskThenTags(t *testing.T) { + defer stubReset() + stubSet("a", StubFixture{Changed: true}) + stubSet("b", StubFixture{Changed: true}) + stubSet("c", StubFixture{Changed: true}) + + path := writeTasksFile(t, `--- +- tasks: + - name: first + tags: [x] + dokku_stub: { key: a } + - name: second + tags: [y] + dokku_stub: { key: b } + - name: third + tags: [x] + dokku_stub: { key: c } +`) + stdout, stderr, exit := runApply(t, path, "--start-at-task", "second", "--tags", "x") + if exit != 0 { + t.Fatalf("exit = %d, want 0; stdout=%s stderr=%s", exit, stdout, stderr) + } + if !strings.Contains(stdout, "[skipped] first (before --start-at-task)") { + t.Errorf("first should be skipped before --start-at-task; got:\n%s", stdout) + } + // second is the resume target but carries tag y, which --tags x + // excludes: it establishes the resume point but does not run and + // leaves no output line. + if strings.Contains(stdout, "second") { + t.Errorf("second should be excluded by --tags and not appear; got:\n%s", stdout) + } + // third is the regression guard: the run must not silently no-op. + if !strings.Contains(stdout, "[changed] third") { + t.Errorf("third should run as [changed]; got:\n%s", stdout) + } +} + +// TestApplyStartAtTaskThenSkipTags pins the same ordering for the +// --skip-tags path: the resume target is dropped by --skip-tags but the +// gate still resumes so a later surviving task runs. +func TestApplyStartAtTaskThenSkipTags(t *testing.T) { + defer stubReset() + stubSet("a", StubFixture{Changed: true}) + stubSet("b", StubFixture{Changed: true}) + stubSet("c", StubFixture{Changed: true}) + + path := writeTasksFile(t, `--- +- tasks: + - name: first + tags: [x] + dokku_stub: { key: a } + - name: second + tags: [y] + dokku_stub: { key: b } + - name: third + tags: [x] + dokku_stub: { key: c } +`) + stdout, stderr, exit := runApply(t, path, "--start-at-task", "second", "--skip-tags", "y") + if exit != 0 { + t.Fatalf("exit = %d, want 0; stdout=%s stderr=%s", exit, stdout, stderr) + } + if !strings.Contains(stdout, "[skipped] first (before --start-at-task)") { + t.Errorf("first should be skipped before --start-at-task; got:\n%s", stdout) + } + if strings.Contains(stdout, "second") { + t.Errorf("second should be dropped by --skip-tags and not appear; got:\n%s", stdout) + } + if !strings.Contains(stdout, "[changed] third") { + t.Errorf("third should run as [changed]; got:\n%s", stdout) + } +} + +// TestApplyStartAtTaskGroupChildWithTags confirms that a resume target +// nested inside a group stays reachable even when the group's own tags +// fail the filter: descent into the resume-point group bypasses tag +// filtering so an explicitly named child is never hidden by --tags. +func TestApplyStartAtTaskGroupChildWithTags(t *testing.T) { + defer stubReset() + stubSet("a", StubFixture{Changed: true}) + stubSet("b", StubFixture{Changed: true}) + + path := writeTasksFile(t, `--- +- tasks: + - name: group-1 + tags: [other] + block: + - name: block-a + dokku_stub: { key: a } + - name: block-b + dokku_stub: { key: b } +`) + stdout, stderr, exit := runApply(t, path, "--start-at-task", "block-b", "--tags", "x") + if exit != 0 { + t.Fatalf("exit = %d, want 0; stdout=%s stderr=%s", exit, stdout, stderr) + } + if !strings.Contains(stdout, "[skipped] [block] block-a (before --start-at-task)") { + t.Errorf("block-a should be skipped before --start-at-task; got:\n%s", stdout) + } + if !strings.Contains(stdout, "[changed] [block] block-b") { + t.Errorf("block-b should run despite the group's tag not matching --tags; got:\n%s", stdout) + } +} diff --git a/docs/command-reference.md b/docs/command-reference.md index 542a4e6..156e98c 100644 --- a/docs/command-reference.md +++ b/docs/command-reference.md @@ -259,7 +259,9 @@ Summary: 4 tasks · 1 changed · 1 ok · 2 skipped · 0 errors (took 1.1s) Filters apply in this order: `--start-at-task` selects first, then `--tags` / `--skip-tags`, then per-task `when:` at execution time. The name search walks every play in source order, narrowed by -`--play`. An unmatched name exits 1 with the available names listed. +`--play`. An unmatched name exits 1 with the available names listed. A `--start-at-task` target that +is itself excluded by `--tags` / `--skip-tags` still establishes the resume point but does not itself +run - tasks after it that pass the tag filter do. ## docket export diff --git a/docs/task-envelope.md b/docs/task-envelope.md index 035261a..c11b940 100644 --- a/docs/task-envelope.md +++ b/docs/task-envelope.md @@ -53,6 +53,11 @@ docket plan --tasks tasks.yml --tags api # only the api task docket apply --tasks tasks.yml --skip-tags worker # everything except worker ``` +When combined with `--start-at-task`, the resume point is selected first and tags narrow the +survivors: `--start-at-task` skips every task before the named one, then `--tags` / `--skip-tags` +filter what remains. See [command reference](command-reference.md#inspecting-and-resuming) for the +full filter order. + ## `when`: run a task conditionally `when:` is an expr expression evaluated for each task just before it runs. A false result renders diff --git a/tasks/envelope.go b/tasks/envelope.go index 8e1cafd..9e8dddf 100644 --- a/tasks/envelope.go +++ b/tasks/envelope.go @@ -169,6 +169,26 @@ func (e *TaskEnvelope) IntersectsTags(tags []string) bool { return false } +// EnvelopePassesTags reports whether env survives the include (--tags) +// and skip (--skip-tags) filters, using the same rules FilterByTags +// applies per key: +// +// - No flags supplied: always kept. +// - --tags supplied: kept iff env's tag set intersects includes; +// untagged envelopes are excluded. +// - --skip-tags supplied: dropped iff env's tag set intersects skips; +// untagged envelopes are kept. +// - Both: --tags narrows first, then --skip-tags drops. +func EnvelopePassesTags(env *TaskEnvelope, includes, skips []string) bool { + if len(includes) > 0 && !env.IntersectsTags(includes) { + return false + } + if len(skips) > 0 && env.IntersectsTags(skips) { + return false + } + return true +} + // FilterByTags returns the subset of m's keys that satisfy the include // (--tags) and skip (--skip-tags) filters. Rules: // @@ -188,16 +208,9 @@ func FilterByTags(m OrderedStringEnvelopeMap, includes, skips []string) []string out := make([]string, 0, len(keys)) for _, k := range keys { - env := m.GetEnvelope(k) - if len(includes) > 0 { - if !env.IntersectsTags(includes) { - continue - } - } - if len(skips) > 0 && env.IntersectsTags(skips) { - continue + if EnvelopePassesTags(m.GetEnvelope(k), includes, skips) { + out = append(out, k) } - out = append(out, k) } return out } diff --git a/tasks/envelope_test.go b/tasks/envelope_test.go index 95c97fb..49323fb 100644 --- a/tasks/envelope_test.go +++ b/tasks/envelope_test.go @@ -98,6 +98,41 @@ func TestFilterByTagsCombinedNarrowsThenDrops(t *testing.T) { } } +// TestEnvelopePassesTags covers the per-envelope predicate that backs +// FilterByTags and the apply loop's post-gate tag filter: no flags keep +// everything, --tags requires an intersection (dropping untagged), and +// --skip-tags drops an intersection (keeping untagged), with both +// combining as narrow-then-drop. +func TestEnvelopePassesTags(t *testing.T) { + tagged := &TaskEnvelope{Tags: []string{"foo", "skip"}} + untagged := &TaskEnvelope{} + + cases := []struct { + name string + env *TaskEnvelope + includes []string + skips []string + want bool + }{ + {"no flags keeps tagged", tagged, nil, nil, true}, + {"no flags keeps untagged", untagged, nil, nil, true}, + {"include intersects", tagged, []string{"foo"}, nil, true}, + {"include misses", tagged, []string{"bar"}, nil, false}, + {"include excludes untagged", untagged, []string{"foo"}, nil, false}, + {"skip intersects drops", tagged, nil, []string{"skip"}, false}, + {"skip misses keeps", tagged, nil, []string{"bar"}, true}, + {"skip keeps untagged", untagged, nil, []string{"skip"}, true}, + {"both narrows then drops", tagged, []string{"foo"}, []string{"skip"}, false}, + {"both keeps when include hits and skip misses", tagged, []string{"foo"}, []string{"bar"}, true}, + } + for _, tc := range cases { + if got := EnvelopePassesTags(tc.env, tc.includes, tc.skips); got != tc.want { + t.Errorf("%s: EnvelopePassesTags(%v, includes=%v, skips=%v) = %v, want %v", + tc.name, tc.env.Tags, tc.includes, tc.skips, got, tc.want) + } + } +} + // TestEnvelopeContainsName covers the helper used by --start-at-task // gating: a top-level name match, a recursive match through a block / // rescue / always child, and a miss.