Add excludeChannels to Slack spawner matching

[knechtionscoding]

What type of PR is this?

/kind api

What this PR does / why we need it:

Adds spec.when.slack.excludeChannels to TaskSpawner: a channel blocklist for Slack spawners.

An excluded channel is never matched — even when channels is empty (all channels) or lists the same channel. Exclusion always wins over the allowlist. Unlike excludePatterns, the exclusion also applies to slash commands, because the channel gate runs before the slash-command bypass in MatchesSpawner.

This makes it possible to carve specific channels out of a catch-all Slack spawner, so channel-scoped spawners can own those channels without the catch-all firing as well. Today the only way to get that is to convert the catch-all into an explicit allowlist and keep it in sync with every channel the bot is invited to.

Changes:

• API (api/v1alpha2/taskspawner_types.go): ExcludeChannels []string on Slack, MaxItems=64 with the same ^[CG][A-Z0-9]{8,}$ item pattern as Channels, plus regenerated deepcopy and CRDs.
• Matcher (internal/slack/filter.go): matchesChannel(channelID, allowed, excluded); exclusion is checked before the allowlist and before the slash-command bypass.
• Conversion (internal/conversion/taskspawner.go): the field is v1alpha2-only, preserved across a v1alpha1 round-trip via the kelos.dev/v1alpha2-slack-exclude-channels annotation, mirroring the existing nameTemplate / githubAppAuth / credentials / commentsReporting pattern. Malformed annotation data is ignored rather than failing conversion.
• CLI (internal/cli/printer.go): kelos get taskspawner <name> detail output shows Exclude Channels, so an operator can see why a spawner did not fire in a channel.
• Docs (docs/reference.md): new row for the field.

Which issue(s) this PR is related to:

N/A

Special notes for your reviewer:

• The field is optional and added only to v1alpha2 (the served/storage version); v1alpha1's schema and Go types are untouched, so existing manifests continue to apply unchanged.
• The item pattern is ^[CGD][A-Z0-9]{8,}$, one character wider than channels' ^[CG].... DMs do reach the matcher — handleMessageEvent has no channel-type gate, so with channels empty a D... channel matches — and for an allowlist the missing D is harmless (omitting a DM excludes it) while for a blocklist it would be an expressiveness hole in exactly the field reached for to stop unwanted spawns. Loosening a brand-new field is safe now in a way tightening it later would not be.
• +listType=set is declared on the new field (duplicate rejection, cleaner server-side-apply merges). I deliberately did not add it to the existing channels field in this PR: on a field with live objects it changes SSA merge behavior and starts rejecting duplicates that were previously accepted. Happy to do it as a follow-up if you want it.
• I considered a CEL rule rejecting channels ∩ excludeChannels overlap and decided against it: overlap is a defined, useful semantic (keep a stable allowlist and mute one of its channels) rather than always a mistake, and a blanket rejection would forbid the precedence this field documents. The reference table states exclusion-wins explicitly instead.
• v1alpha1 does not gain the capability. A client that reads and writes a spawner through v1alpha1 will not see excludeChannels, but the value is not silently dropped — it round-trips through the preservation annotation. Because this field fails toward more behavior if that annotation is ever lost (unlike nameTemplate/credentials/gatewayRefs, which fail toward less), both the godoc and the reference row say the exclusion is only guaranteed while the object is managed through v1alpha2.
• The API server does not re-validate the output of a conversion webhook — I confirmed this in envtest: a v1alpha1 create carrying a hand-written ["lower-case-and-way-too-long!!","X"] annotation succeeded and the stored v1alpha2 object came back holding both values. So the restore path now checks the annotation payload against the field's own constraints (item pattern, maxItems, set uniqueness) and ignores it entirely when it fails, rather than partially applying it. Worth noting the sibling preserved fields (githubAppAuth, credentials, gatewayRefs, nameTemplate) share this property; I left them alone to keep this PR scoped.
• All CRD YAML is make update output, not hand-edited.
• Tests: matcher precedence (allowlist vs blocklist, empty allowlist, slash commands, DMs, and the positive non-excluded path), conversion round-trip plus the malformed-annotation and cleared-annotation branches, the full Exclude Channels: [...] line in the printer test, and an envtest spec in test/integration/conversion_test.go that creates the spawner in v1alpha2 (proving the real API server accepts the field, including a D... value), reads it as v1alpha1, writes it back through v1alpha1, and asserts excludeChannels is restored and the annotation cleaned up. The write is wrapped in the suite's conflict-tolerant Eventually idiom because the TaskSpawner controller reconciles the object concurrently.
• Verified locally: make verify, make test, and the full ./test/integration/ suite pass.

Does this PR introduce a user-facing change?

Added `spec.when.slack.excludeChannels` to TaskSpawner — a Slack channel blocklist that wins over the `channels` allowlist and also applies to slash commands, so a catch-all spawner can be excluded from specific channels.

[cubic-dev-ai]

No issues found across 12 files

_{Re-trigger cubic}

[cubic-dev-ai]

1 issue found and verified against the latest diff

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/conversion/taskspawner.go">

<violation number="1" location="internal/conversion/taskspawner.go:214">
P2: When a user supplies valid JSON containing an invalid ID or more than 64 entries, this branch restores it into `v1alpha2` without enforcing the field schema. Validate the item count and channel-ID pattern before assignment so a v1alpha1 round-trip cannot produce schema-invalid hub data or bypass these constraints.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

2 issues found across 12 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/conversion/taskspawner.go">

<violation number="1" location="internal/conversion/taskspawner.go:219">
P2: When a v1alpha1 object carries valid JSON with invalid channel IDs or more than 64 entries, this assignment bypasses the v1alpha2 `Pattern` and `MaxItems` constraints. Validate every restored value against the field constraints or ignore the annotation, so conversion cannot produce invalid hub objects or ineffective exclusions.</violation>
</file>

<file name="test/integration/conversion_test.go">

<violation number="1" location="test/integration/conversion_test.go:178">
P3: The test creates namespace test-conv-ts-slack and TaskSpawner ts-slack but never cleans them up; the AfterAll only removes AgentConfig objects in test-conv-up/test-conv-down. The TaskSpawner controller adds the kelos.dev/taskspawner-finalizer, so leftover instances (this one and test-conv-ts from the test above) persist and can block later install/uninstall specs that delete the taskspawner CRD. Delete the TaskSpawner and namespace in AfterAll (or via DeferCleanup) alongside the AgentConfig cleanup.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

1 issue found across 12 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/manifests/install-crd.yaml">

<violation number="1" location="internal/manifests/install-crd.yaml:60643">
P2: The new excludeChannels field has x-kubernetes-list-type: set to prevent duplicates, but the related channels field lacks this marker. Both fields represent sets of unique channel IDs where duplicates serve no purpose. This inconsistency makes the API design less uniform. Consider adding the same list-type marker to the channels field in the source types (api/v1alpha2/taskspawner_types.go) and regenerating the CRD.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: The new excludeChannels field has x-kubernetes-list-type: set to prevent duplicates, but the related channels field lacks this marker. Both fields represent sets of unique channel IDs where duplicates serve no purpose. This inconsistency makes the API design less uniform. Consider adding the same list-type marker to the channels field in the source types (api/v1alpha2/taskspawner_types.go) and regenerating the CRD.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At internal/manifests/install-crd.yaml, line 60643:

<comment>The new excludeChannels field has x-kubernetes-list-type: set to prevent duplicates, but the related channels field lacks this marker. Both fields represent sets of unique channel IDs where duplicates serve no purpose. This inconsistency makes the API design less uniform. Consider adding the same list-type marker to the channels field in the source types (api/v1alpha2/taskspawner_types.go) and regenerating the CRD.</comment>

<file context>
@@ -60620,6 +60620,27 @@ spec:
+                          type: string
+                        maxItems: 64
+                        type: array
+                        x-kubernetes-list-type: set
                       excludePatterns:
                         description: |-
</file context>