feat: per-task Slack notifications with configurable filters

[adamrtalbot]

Summary

• Add slack.onTaskComplete with Nextflow-style withName/withLabel selectors, per-selector minDuration, onFirstFailure, and throttleInterval
• Hook SlackObserver.onProcessComplete to send Block Kit task messages (process, duration, exit, peak memory/CPU, work dir) as thread replies when threading is enabled
• Reuse Nextflow-compatible selector matching semantics (regex patterns, negation)

Closes #34

Test plan

• ./gradlew test
• Configure onTaskComplete with withName selector against a long-running process
• Verify throttling skips rapid consecutive notifications
• Verify onFirstFailure sends on first failed task

Made with Cursor

[claude]

Code Review

This PR adds slack.onTaskComplete with Nextflow-style withName/withLabel selectors, per-selector minDuration, onFirstFailure, and a throttle — a solid feature overall. The tests cover the main selector and throttle paths. A few correctness issues need attention before merging.

---

Bugs

firstTaskFailureNotified is not thread-safe
SlackObserver.groovy — firstTaskFailureNotified is a plain boolean field, but onProcessComplete is called concurrently from executor threads. Two simultaneous failures both read false, both pass shouldNotify, and both send a "first failure" notification before either sets the flag. Replace with AtomicBoolean and use compareAndSet(false, true) to make the first-failure guard atomic.

PatternSyntaxException escapes unhandled
SlackObserver.groovy — The call to TaskNotificationMatcher.shouldNotify(...) (line ~322) is outside the try/catch block that starts a few lines later. Pattern.compile(pattern) inside the matcher throws PatternSyntaxException (an unchecked IllegalArgumentException) if a user writes a malformed regex (e.g., withName: '*ALIGN'). This exception propagates out of onProcessComplete unguarded, breaking notifications for every subsequent task. Move shouldNotify inside the try block or add a catch at the call site.

onFirstFailure bypasses the throttle gate
SlackObserver.groovy — shouldNotify returns true early for the first-failure path, before the throttle check runs. This means a first-failure notification is always sent even if a throttled notification was just emitted moments earlier. If both onFirstFailure and a selector are active, both checks fire independently of each other and of the throttle window.

Seqera Platform button polling timers are never cancelled
SlackObserver.groovy — scheduleSeqeraPlatformButtonUpdateAttempt creates new Timer(...) as a local variable (no field assignment) on each retry attempt — up to 15 times. There is no reference stored and no cancellation in cancelProgressTimer(). These timers survive the workflow and fire sender.updateMessage() for up to 30 s after onFlowComplete/onFlowError, racing with teardown and reaction cleanup.

---

Missing

buildTaskCompleteMessage omits the Seqera Platform button
SlackMessageBuilder.groovy — buildWorkflowCompleteMessage and buildProgressUpdateMessage both append the Seqera Platform deep-link button when available. buildTaskCompleteMessage does not call getSeqeraPlatformUrl() at all, so per-task notifications silently lack the button even when seqeraPlatform.enabled = true.

No docs or example configuration
The example/ directory and any docs site are not updated. onTaskComplete defaults to enabled = false (unlike all other on* configs which default to enabled = true), and there is no warning logged when selectors are configured but enabled is not set. This combination will generate user confusion. At minimum, an example showing a minimal working config and noting the enabled: true requirement is needed.

[claude]

Code Review

This PR adds slack.onTaskComplete with Nextflow-style withName/withLabel selectors, per-selector minDuration, onFirstFailure, and throttling. The design is solid and the new config parsing classes are well-structured. A few bugs and one missing piece need attention before merge.

---

Bugs

1. firstTaskFailureNotified is a plain boolean — race condition
SlackObserver.groovy (field declared as private boolean firstTaskFailureNotified = false).
onProcessComplete is called concurrently from multiple Nextflow executor threads. Two simultaneous task failures both read false, both pass the !firstFailureAlreadySent guard, and both send the "first failure" notification. The flag that was meant to deduplicate it fires it twice.

Fix: private final AtomicBoolean firstTaskFailureNotified = new AtomicBoolean(false) and guard with firstTaskFailureNotified.compareAndSet(false, true).

---

2. matchesLabels returns true for tasks with no labels when using a negated pattern
OnTaskCompleteConfig.groovy lines 102–111:

for (String label : labels ?: Collections.<String>emptyList()) {
    if (regex.matcher(label).matches()) return !isNegated
}
return isNegated   // <-- true when isNegated=true, labels=[]

A task with zero labels matches any withLabel: '!gpu' rule and receives a spurious notification.

Fix: the final return should be return false — a task with no labels should not match any label selector, negated or not.

---

3. PatternSyntaxException from a bad user-supplied regex is not caught
matchesName and matchesLabels call Pattern.compile(pattern) without wrapping in a try/catch. In maybeSendTaskCompleteNotification, the call to shouldNotify (which calls these methods) is outside the try/catch block that guards sendMessage. A config typo like withName: 'ALIGN[BAD' will throw an uncaught exception into onProcessComplete, which can abort Nextflow's task-tracking machinery.

Fix: wrap Pattern.compile(pattern) in try/catch PatternSyntaxException (log a warning and return false), or catch it in maybeSendTaskCompleteNotification.

---

4. Non-atomic throttle check-and-set
SlackObserver.groovy — the throttle guard reads lastTaskNotificationTime.get(), compares, then calls sender.sendMessage(...), then calls lastTaskNotificationTime.set(now). With concurrent task completions, two threads can both pass the comparison before either one sets the new value, bypassing the throttle.

Fix: use a synchronized block or AtomicLong.compareAndSet to make the check and update atomic.

---

Minor

5. New Timer per polling attempt leaks threads
scheduleSeqeraPlatformButtonUpdateAttempt creates new Timer(...) on each of up to 15 recursive calls. At peak that's 15 daemon threads alive simultaneously. A single ScheduledExecutorService with fixed-rate scheduling would handle this with one thread.

---

Documentation

6. No documentation or example config for onTaskComplete
The PR body's manual test checklist has unchecked boxes, and no example/ config or README section documents the new onTaskComplete block, its keys, or selector syntax. Per the project constitution ("Documentation as Code"), public API surface needs docs before merge.

[claude]

Code Review

This PR adds slack.onTaskComplete with Nextflow-style withName/withLabel selectors, per-selector minDuration, onFirstFailure, and throttling. The feature is well-structured and tests cover the main paths — but there are two concurrency bugs that will cause duplicate notifications in real workflows, plus missing documentation.

---

Findings (most-severe first)

[Bug] firstTaskFailureNotified is a plain boolean — race condition
SlackObserver.groovy ~line 63

private boolean firstTaskFailureNotified = false has no volatile modifier and no synchronization. onProcessComplete is called from multiple executor threads concurrently. Two simultaneously failing tasks will both read false, both pass shouldNotify, and both fire an "on first failure" notification — defeating the deduplication entirely.

Fix: replace with private final AtomicBoolean firstTaskFailureNotified = new AtomicBoolean(false) and use compareAndSet(false, true) to gate the send.

---

[Bug] Throttle check-then-set is not atomic
SlackObserver.groovy ~line 455 (maybeSendTaskCompleteNotification)

if (lastTaskNotificationTime.get() > 0 && (now - lastTaskNotificationTime.get()) < taskThrottleIntervalMs) return
// ...send...
lastTaskNotificationTime.set(now)

AtomicLong makes individual operations atomic, not the read-check-send-set sequence. Two concurrent task completions will both read the old value, both pass the guard, and both send. Use AtomicLong.compareAndSet or a synchronized block around the check+set.

---

[Bug] failedTasks is over-counted for retried tasks
SlackObserver.groovy ~line 137 (onProcessComplete)

Before this PR, failedTasks was only written by reconcileCountsFromMetadata() at workflow end from session.workflowMetadata.stats.failedCount, which counts only tasks that ultimately failed. The PR adds a live increment on every non-zero exit code. A task that fails once then is retried successfully will increment failedTasks even though the final workflow count is 0. When onProgress is disabled (the default), reconcileCountsFromMetadata() is never called to correct this.

---

[Bug] Timer thread leak in Seqera button polling
SlackObserver.groovy ~line 185 (scheduleSeqeraPlatformButtonUpdateAttempt)

Each recursive retry call creates new Timer('slack-seqera-button', true) — a new daemon thread each time. Up to 15 Timer objects (15 threads) are created and never cancelled during the polling window. Reuse a single Timer instance (store it as a field or pass it as a parameter).

---

[Documentation] onTaskComplete has no user-facing documentation

README.md, docs/reference/api.md, and all example/configs/ files contain zero mentions of onTaskComplete, withName:, withLabel:, throttleInterval, or onFirstFailure. Users have no way to discover or configure this feature. The PR test plan also has two unchecked manual-test items.

---

Minor

The parseDurationMillis helper duplicates logic already present in OnProgressConfig. Worth extracting to a shared utility to avoid drift.

[adamrtalbot]

Addressed Claude review in 16dba89:

• AtomicBoolean + synchronized throttle for first-failure dedup and per-task sends.
• matchesLabels: empty-label tasks no longer match negated patterns; invalid regex returns false.
• PatternSyntaxException caught in selector matcher.
• Reuse single seqeraButtonTimer, cancelled in cancelProgressTimer().
• Removed live failedTasks increment; reconcileCountsFromMetadata() called before progress updates.
• Seqera Platform button added to buildTaskCompleteMessage.
• Docs + example/configs/14-per-task-notifications.config added.

./gradlew test passes locally.

[adamrtalbot]

A nice idea but this seems like a lot of complexity.

It might make more sense to extend the core Nextflow plugin extension points to include withName/withLabel filters instead.