From 57967fbeb9aee29b37e3cc60367c122611a84641 Mon Sep 17 00:00:00 2001 From: Alex Kuleshov Date: Sun, 22 Mar 2026 13:08:10 -0400 Subject: [PATCH 1/4] fix(slack): remove legacy allowlists --- .../2026-03-22-slack-remove-allowlists.md | 86 +++++++++++++++++++ ...26-03-22-slack-remove-allowlists-design.md | 34 ++++++++ .../golemcore/slack/SlackPluginConfig.java | 30 ------- .../slack/SlackPluginSettingsContributor.java | 31 ------- .../adapter/inbound/slack/SlackAdapter.java | 25 +----- .../slack/SlackPluginConfigServiceTest.java | 14 +-- .../slack/SlackPluginConfigTest.java | 8 +- .../SlackPluginSettingsContributorTest.java | 17 ++-- .../inbound/slack/SlackAdapterTest.java | 12 ++- 9 files changed, 148 insertions(+), 109 deletions(-) create mode 100644 docs/superpowers/plans/2026-03-22-slack-remove-allowlists.md create mode 100644 docs/superpowers/specs/2026-03-22-slack-remove-allowlists-design.md diff --git a/docs/superpowers/plans/2026-03-22-slack-remove-allowlists.md b/docs/superpowers/plans/2026-03-22-slack-remove-allowlists.md new file mode 100644 index 0000000..05acd4f --- /dev/null +++ b/docs/superpowers/plans/2026-03-22-slack-remove-allowlists.md @@ -0,0 +1,86 @@ +# Slack Remove Allowlists Implementation Plan + +> **For agentic workers:** REQUIRED: Use superpowers:subagent-driven-development (if subagents available) or superpowers:executing-plans to implement this plan. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Remove Slack user/channel allowlists from config, UI, and runtime filtering while keeping legacy stored keys harmless. + +**Architecture:** Keep the persisted-config compatibility boundary in `SlackPluginConfig` via unknown-field tolerance, simplify the settings surface, and remove runtime allowlist checks from `SlackAdapter`. The change is intentionally narrow and does not alter mention or thread routing semantics. + +**Tech Stack:** Java 25, Spring, JUnit 5, Mockito, Maven + +--- + +### Task 1: Update Tests For Removed Allowlists + +**Files:** +- Modify: `golemcore/slack/src/test/java/me/golemcore/plugins/golemcore/slack/SlackPluginConfigServiceTest.java` +- Modify: `golemcore/slack/src/test/java/me/golemcore/plugins/golemcore/slack/SlackPluginSettingsContributorTest.java` +- Modify: `golemcore/slack/src/test/java/me/golemcore/plugins/golemcore/slack/adapter/inbound/slack/SlackAdapterTest.java` + +- [ ] **Step 1: Write failing assertions for config-service behavior** + +Remove expectations that `allowedUserIds` and `allowedChannelIds` exist in the runtime config or saved payload, while keeping the legacy input keys in the mocked stored config. + +- [ ] **Step 2: Run the targeted config-service test to verify it fails** + +Run: `mvn -B -ntp -f pom.xml -pl :golemcore-slack-plugin -Dtest=SlackPluginConfigServiceTest test` + +- [ ] **Step 3: Write failing assertions for settings section exposure/save** + +Update the settings contributor tests so the section no longer exposes the allowlist values and save payloads no longer accept or persist them. + +- [ ] **Step 4: Run the targeted settings-contributor test to verify it fails** + +Run: `mvn -B -ntp -f pom.xml -pl :golemcore-slack-plugin -Dtest=SlackPluginSettingsContributorTest test` + +- [ ] **Step 5: Write failing assertions for inbound message handling** + +Update the adapter tests so prior allowlist-style config does not cause inbound messages to be ignored. + +- [ ] **Step 6: Run the targeted adapter test to verify it fails** + +Run: `mvn -B -ntp -f pom.xml -pl :golemcore-slack-plugin -Dtest=SlackAdapterTest test` + +### Task 2: Remove Allowlists From Slack Runtime And Settings + +**Files:** +- Modify: `golemcore/slack/src/main/java/me/golemcore/plugins/golemcore/slack/SlackPluginConfig.java` +- Modify: `golemcore/slack/src/main/java/me/golemcore/plugins/golemcore/slack/SlackPluginSettingsContributor.java` +- Modify: `golemcore/slack/src/main/java/me/golemcore/plugins/golemcore/slack/adapter/inbound/slack/SlackAdapter.java` + +- [ ] **Step 1: Remove allowlist fields and normalization from the config model** + +Delete `allowedUserIds` and `allowedChannelIds` from `SlackPluginConfig` and keep normalization focused on supported fields. + +- [ ] **Step 2: Remove allowlist fields from the settings section and save flow** + +Delete the UI fields, values, and save-path parsing for the two legacy keys. + +- [ ] **Step 3: Remove runtime user/channel allowlist checks** + +Delete the inbound/action filter methods in `SlackAdapter` and keep only the thread/session gating logic. + +- [ ] **Step 4: Run the targeted Slack test class to verify the implementation passes** + +Run: `mvn -B -ntp -f pom.xml -pl :golemcore-slack-plugin -Dtest=SlackPluginConfigServiceTest,SlackPluginSettingsContributorTest,SlackAdapterTest test` + +### Task 3: Verify And Prepare PR + +**Files:** +- Verify only + +- [ ] **Step 1: Run the Slack module package build** + +Run: `mvn -B -ntp -f pom.xml -pl :golemcore-slack-plugin -am package` + +- [ ] **Step 2: Run repository validation** + +Run: `python3 scripts/plugins_repo.py validate` + +- [ ] **Step 3: Commit the change** + +Use: `git add ... && git commit -m "fix(slack): remove legacy allowlists"` + +- [ ] **Step 4: Push branch and open PR** + +Use: `git push -u origin fix/slack-remove-allowlists` then create the PR against `main`. diff --git a/docs/superpowers/specs/2026-03-22-slack-remove-allowlists-design.md b/docs/superpowers/specs/2026-03-22-slack-remove-allowlists-design.md new file mode 100644 index 0000000..ade3844 --- /dev/null +++ b/docs/superpowers/specs/2026-03-22-slack-remove-allowlists-design.md @@ -0,0 +1,34 @@ +# Slack Remove Allowlists Design + +## Goal + +Remove `allowedUserIds` and `allowedChannelIds` from the Slack plugin's public configuration and runtime behavior so Slack message intake depends only on Slack event type, bot installation, and existing thread/session rules. + +## Scope + +- Remove both allowlist fields from the Slack config model. +- Remove both fields from the Slack settings UI and save flow. +- Remove inbound user/channel filtering from the Slack adapter. +- Continue tolerating legacy persisted config keys silently. + +## Non-Goals + +- No storage migration job. +- No change to mention-only behavior for top-level channel messages. +- No change to thread/session routing. + +## Design + +The plugin will keep `@JsonIgnoreProperties(ignoreUnknown = true)` on the config model, which lets older saved configs still deserialize cleanly even after the Java fields are removed. The settings contributor will stop exposing and saving the legacy keys, so any subsequent save naturally rewrites the stored config without them. + +Inbound Slack handling will no longer call allowlist-based user/channel filters. The adapter will continue to enforce the existing event-shape rules: + +- DMs are accepted. +- `app_mention` events are accepted. +- thread replies are accepted only when they belong to an active conversation. + +## Tests + +- Update config-service tests to verify legacy allowlist keys are ignored on read and are not persisted on save. +- Update settings-contributor tests to verify the section no longer exposes allowlist fields and save keeps only supported values. +- Update adapter tests to verify inbound messages are not dropped because of prior allowlist configuration. diff --git a/golemcore/slack/src/main/java/me/golemcore/plugins/golemcore/slack/SlackPluginConfig.java b/golemcore/slack/src/main/java/me/golemcore/plugins/golemcore/slack/SlackPluginConfig.java index 6913d20..8d2c0de 100644 --- a/golemcore/slack/src/main/java/me/golemcore/plugins/golemcore/slack/SlackPluginConfig.java +++ b/golemcore/slack/src/main/java/me/golemcore/plugins/golemcore/slack/SlackPluginConfig.java @@ -7,11 +7,6 @@ import lombok.Data; import lombok.NoArgsConstructor; -import java.util.ArrayList; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Set; - @JsonIgnoreProperties(ignoreUnknown = true) @Data @NoArgsConstructor @@ -28,12 +23,6 @@ public class SlackPluginConfig { @Builder.Default private Boolean replyInThread = true; - @Builder.Default - private List allowedUserIds = new ArrayList<>(); - - @Builder.Default - private List allowedChannelIds = new ArrayList<>(); - public void normalize() { if (enabled == null) { enabled = false; @@ -43,8 +32,6 @@ public void normalize() { } botToken = normalizeSecret(botToken); appToken = normalizeSecret(appToken); - allowedUserIds = normalizeIdentifiers(allowedUserIds); - allowedChannelIds = normalizeIdentifiers(allowedChannelIds); } @JsonIgnore @@ -60,21 +47,4 @@ private String normalizeSecret(String value) { String normalized = value.trim(); return normalized.isBlank() ? null : normalized; } - - private List normalizeIdentifiers(List values) { - if (values == null || values.isEmpty()) { - return List.of(); - } - Set normalized = new LinkedHashSet<>(); - for (String value : values) { - if (value == null) { - continue; - } - String candidate = value.trim(); - if (!candidate.isBlank()) { - normalized.add(candidate); - } - } - return List.copyOf(normalized); - } } diff --git a/golemcore/slack/src/main/java/me/golemcore/plugins/golemcore/slack/SlackPluginSettingsContributor.java b/golemcore/slack/src/main/java/me/golemcore/plugins/golemcore/slack/SlackPluginSettingsContributor.java index 76ab8c9..84935fe 100644 --- a/golemcore/slack/src/main/java/me/golemcore/plugins/golemcore/slack/SlackPluginSettingsContributor.java +++ b/golemcore/slack/src/main/java/me/golemcore/plugins/golemcore/slack/SlackPluginSettingsContributor.java @@ -56,8 +56,6 @@ public PluginSettingsSection getSection(String sectionKey) { values.put("botToken", ""); values.put("appToken", ""); values.put("replyInThread", Boolean.TRUE.equals(config.getReplyInThread())); - values.put("allowedUserIds", String.join(", ", config.getAllowedUserIds())); - values.put("allowedChannelIds", String.join(", ", config.getAllowedChannelIds())); return PluginSettingsSection.builder() .title("Slack") @@ -92,21 +90,6 @@ public PluginSettingsSection getSection(String sectionKey) { .label("Reply In Threads") .description( "Route channel mentions into one session per root thread and answer in that thread.") - .build(), - PluginSettingsField.builder() - .key("allowedUserIds") - .type("text") - .label("Allowed User IDs") - .description("Optional comma-separated Slack user ids. Leave blank to allow any user.") - .placeholder("U01234567, U08999999") - .build(), - PluginSettingsField.builder() - .key("allowedChannelIds") - .type("text") - .label("Allowed Channel IDs") - .description( - "Optional comma-separated Slack channel ids. Leave blank to allow any channel or DM.") - .placeholder("C01234567, D08999999") .build())) .values(values) .blocks(List.of(PluginSettingsBlock.builder() @@ -141,9 +124,6 @@ public PluginSettingsSection saveSection(String sectionKey, Map config.setAppToken(appToken); } - config.setAllowedUserIds(parseCsv(readString(values, "allowedUserIds", ""))); - config.setAllowedChannelIds(parseCsv(readString(values, "allowedChannelIds", ""))); - configService.save(config); eventPublisher.publishEvent(new SlackRestartEvent()); return getSection(sectionKey); @@ -177,15 +157,4 @@ private String readString(Map values, String key, String default Object value = values.get(key); return value instanceof String text ? text : defaultValue; } - - private List parseCsv(String raw) { - if (raw == null || raw.isBlank()) { - return List.of(); - } - return java.util.Arrays.stream(raw.split(",")) - .map(String::trim) - .filter(value -> !value.isBlank()) - .distinct() - .toList(); - } } diff --git a/golemcore/slack/src/main/java/me/golemcore/plugins/golemcore/slack/adapter/inbound/slack/SlackAdapter.java b/golemcore/slack/src/main/java/me/golemcore/plugins/golemcore/slack/adapter/inbound/slack/SlackAdapter.java index bc22061..1e508f0 100644 --- a/golemcore/slack/src/main/java/me/golemcore/plugins/golemcore/slack/adapter/inbound/slack/SlackAdapter.java +++ b/golemcore/slack/src/main/java/me/golemcore/plugins/golemcore/slack/adapter/inbound/slack/SlackAdapter.java @@ -180,8 +180,7 @@ public CompletableFuture sendProgressUpdate(String chatId, ProgressUpdate @Override public boolean isAuthorized(String senderId) { - SlackPluginConfig config = configService.getConfig(); - return config.getAllowedUserIds().isEmpty() || config.getAllowedUserIds().contains(senderId); + return true; } @Override @@ -214,14 +213,6 @@ private void handleInbound(SlackInboundEnvelope envelope) { if (envelope == null) { return; } - if (!isAuthorized(envelope.userId())) { - log.warn("[Slack] Ignoring message from unauthorized user {}", envelope.userId()); - return; - } - if (!isChannelAllowed(envelope.channelId())) { - log.warn("[Slack] Ignoring message from unauthorized channel {}", envelope.channelId()); - return; - } if (requiresExistingConversation(envelope) && !hasExistingConversation(envelope)) { log.debug("[Slack] Ignoring thread reply without active conversation {}", envelope.rootThreadTs()); return; @@ -239,15 +230,6 @@ private void handleAction(SlackActionEnvelope envelope) { if (envelope == null) { return; } - if (!isAuthorized(envelope.userId())) { - log.warn("[Slack] Ignoring action from unauthorized user {}", envelope.userId()); - return; - } - if (!isChannelAllowed(envelope.channelId())) { - log.warn("[Slack] Ignoring action from unauthorized channel {}", envelope.channelId()); - return; - } - String actionId = envelope.actionId(); if (SlackActionIds.CONFIRM_APPROVE.equals(actionId) || SlackActionIds.CONFIRM_CANCEL.equals(actionId)) { eventPublisher.publishEvent(new ConfirmationCallbackEvent( @@ -266,11 +248,6 @@ private void handleAction(SlackActionEnvelope envelope) { } } - private boolean isChannelAllowed(String channelId) { - SlackPluginConfig config = configService.getConfig(); - return config.getAllowedChannelIds().isEmpty() || config.getAllowedChannelIds().contains(channelId); - } - private Message buildInboundMessage(SlackInboundEnvelope envelope) { SlackConversationTarget target = resolveConversationTarget(envelope); diff --git a/golemcore/slack/src/test/java/me/golemcore/plugins/golemcore/slack/SlackPluginConfigServiceTest.java b/golemcore/slack/src/test/java/me/golemcore/plugins/golemcore/slack/SlackPluginConfigServiceTest.java index 989da9d..990b9f0 100644 --- a/golemcore/slack/src/test/java/me/golemcore/plugins/golemcore/slack/SlackPluginConfigServiceTest.java +++ b/golemcore/slack/src/test/java/me/golemcore/plugins/golemcore/slack/SlackPluginConfigServiceTest.java @@ -5,6 +5,8 @@ import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; +import com.fasterxml.jackson.databind.ObjectMapper; + import java.util.Map; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -18,6 +20,7 @@ class SlackPluginConfigServiceTest { private PluginConfigurationService pluginConfigurationService; private SlackPluginConfigService service; + private final ObjectMapper objectMapper = new ObjectMapper(); @BeforeEach void setUp() { @@ -37,12 +40,13 @@ void shouldIgnoreLegacyConfiguredFieldWhenReadingStoredPluginConfig() { "allowedChannelIds", java.util.List.of("C123"))); SlackPluginConfig config = service.getConfig(); + Map serialized = objectMapper.convertValue(config, Map.class); assertEquals("xoxb-test", config.getBotToken()); assertEquals("xapp-test", config.getAppToken()); assertFalse(Boolean.TRUE.equals(config.getReplyInThread())); - assertEquals(java.util.List.of("U123"), config.getAllowedUserIds()); - assertEquals(java.util.List.of("C123"), config.getAllowedChannelIds()); + assertFalse(serialized.containsKey("allowedUserIds")); + assertFalse(serialized.containsKey("allowedChannelIds")); } @Test @@ -52,8 +56,6 @@ void shouldPersistNormalizedPluginConfigWithoutComputedFields() { .botToken(" xoxb-token ") .appToken(" xapp-token ") .replyInThread(null) - .allowedUserIds(java.util.List.of(" U123 ", "", "U123")) - .allowedChannelIds(java.util.List.of(" C123 ", "C123")) .build(); service.save(config); @@ -66,8 +68,8 @@ void shouldPersistNormalizedPluginConfigWithoutComputedFields() { assertEquals("xapp-token", saved.get("appToken")); assertEquals(true, saved.get("enabled")); assertEquals(true, saved.get("replyInThread")); - assertEquals(java.util.List.of("U123"), saved.get("allowedUserIds")); - assertEquals(java.util.List.of("C123"), saved.get("allowedChannelIds")); + assertFalse(saved.containsKey("allowedUserIds")); + assertFalse(saved.containsKey("allowedChannelIds")); assertFalse(saved.containsKey("configured")); } } diff --git a/golemcore/slack/src/test/java/me/golemcore/plugins/golemcore/slack/SlackPluginConfigTest.java b/golemcore/slack/src/test/java/me/golemcore/plugins/golemcore/slack/SlackPluginConfigTest.java index 2c9ca2d..ec0fc89 100644 --- a/golemcore/slack/src/test/java/me/golemcore/plugins/golemcore/slack/SlackPluginConfigTest.java +++ b/golemcore/slack/src/test/java/me/golemcore/plugins/golemcore/slack/SlackPluginConfigTest.java @@ -2,8 +2,6 @@ import org.junit.jupiter.api.Test; -import java.util.List; - import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -11,14 +9,12 @@ class SlackPluginConfigTest { @Test - void shouldNormalizeDefaultsAndIdentifiers() { + void shouldNormalizeDefaultsAndSecrets() { SlackPluginConfig config = SlackPluginConfig.builder() .enabled(null) .replyInThread(null) .botToken(" ") .appToken(" xapp-token ") - .allowedUserIds(List.of(" U123 ", "", "U123", "U999")) - .allowedChannelIds(List.of(" C123 ", "C123", "D456")) .build(); config.normalize(); @@ -27,7 +23,5 @@ void shouldNormalizeDefaultsAndIdentifiers() { assertTrue(Boolean.TRUE.equals(config.getReplyInThread())); assertNull(config.getBotToken()); assertEquals("xapp-token", config.getAppToken()); - assertEquals(List.of("U123", "U999"), config.getAllowedUserIds()); - assertEquals(List.of("C123", "D456"), config.getAllowedChannelIds()); } } diff --git a/golemcore/slack/src/test/java/me/golemcore/plugins/golemcore/slack/SlackPluginSettingsContributorTest.java b/golemcore/slack/src/test/java/me/golemcore/plugins/golemcore/slack/SlackPluginSettingsContributorTest.java index 9ba26fc..e2108bc 100644 --- a/golemcore/slack/src/test/java/me/golemcore/plugins/golemcore/slack/SlackPluginSettingsContributorTest.java +++ b/golemcore/slack/src/test/java/me/golemcore/plugins/golemcore/slack/SlackPluginSettingsContributorTest.java @@ -1,5 +1,6 @@ package me.golemcore.plugins.golemcore.slack; +import com.fasterxml.jackson.databind.ObjectMapper; import me.golemcore.plugin.api.extension.spi.PluginActionResult; import me.golemcore.plugin.api.extension.spi.PluginSettingsSection; import org.junit.jupiter.api.BeforeEach; @@ -22,6 +23,7 @@ class SlackPluginSettingsContributorTest { private ApplicationEventPublisher eventPublisher; private SlackPluginSettingsContributor contributor; private SlackPluginConfig config; + private final ObjectMapper objectMapper = new ObjectMapper(); @BeforeEach void setUp() { @@ -33,8 +35,6 @@ void setUp() { .botToken("xoxb-existing") .appToken("xapp-existing") .replyInThread(true) - .allowedUserIds(List.of("U123")) - .allowedChannelIds(List.of("C123")) .build(); when(configService.getConfig()).thenReturn(config); } @@ -45,8 +45,8 @@ void shouldExposeSectionWithBlankSecrets() { assertEquals("", section.getValues().get("botToken")); assertEquals("", section.getValues().get("appToken")); - assertEquals("U123", section.getValues().get("allowedUserIds")); - assertEquals("C123", section.getValues().get("allowedChannelIds")); + assertTrue(!section.getValues().containsKey("allowedUserIds")); + assertTrue(!section.getValues().containsKey("allowedChannelIds")); } @Test @@ -55,19 +55,18 @@ void shouldPreserveSecretsWhenBlankValuesAreSaved() { "enabled", false, "botToken", "", "appToken", "", - "replyInThread", false, - "allowedUserIds", "U777, U888", - "allowedChannelIds", "D123, C999")); + "replyInThread", false)); ArgumentCaptor captor = ArgumentCaptor.forClass(SlackPluginConfig.class); verify(configService).save(captor.capture()); SlackPluginConfig saved = captor.getValue(); + Map serialized = objectMapper.convertValue(saved, Map.class); assertEquals("xoxb-existing", saved.getBotToken()); assertEquals("xapp-existing", saved.getAppToken()); assertTrue(Boolean.FALSE.equals(saved.getEnabled())); assertTrue(Boolean.FALSE.equals(saved.getReplyInThread())); - assertEquals(List.of("U777", "U888"), saved.getAllowedUserIds()); - assertEquals(List.of("D123", "C999"), saved.getAllowedChannelIds()); + assertTrue(!serialized.containsKey("allowedUserIds")); + assertTrue(!serialized.containsKey("allowedChannelIds")); verify(eventPublisher).publishEvent(org.mockito.ArgumentMatchers.any(SlackRestartEvent.class)); } diff --git a/golemcore/slack/src/test/java/me/golemcore/plugins/golemcore/slack/adapter/inbound/slack/SlackAdapterTest.java b/golemcore/slack/src/test/java/me/golemcore/plugins/golemcore/slack/adapter/inbound/slack/SlackAdapterTest.java index d2ecc41..ac7d51e 100644 --- a/golemcore/slack/src/test/java/me/golemcore/plugins/golemcore/slack/adapter/inbound/slack/SlackAdapterTest.java +++ b/golemcore/slack/src/test/java/me/golemcore/plugins/golemcore/slack/adapter/inbound/slack/SlackAdapterTest.java @@ -1,5 +1,6 @@ package me.golemcore.plugins.golemcore.slack.adapter.inbound.slack; +import com.fasterxml.jackson.databind.ObjectMapper; import me.golemcore.plugin.api.extension.loop.AgentLoop; import me.golemcore.plugin.api.extension.model.ConfirmationCallbackEvent; import me.golemcore.plugin.api.extension.model.ContextAttributes; @@ -48,6 +49,7 @@ class SlackAdapterTest { private ApplicationEventPublisher eventPublisher; private SlackAdapter adapter; private SlackPluginConfig config; + private final ObjectMapper objectMapper = new ObjectMapper(); @BeforeEach void setUp() { @@ -103,7 +105,13 @@ void shouldPublishInboundThreadedMessage() { @Test void shouldIgnoreUnauthorizedUsers() { - config.setAllowedUserIds(List.of("U111")); + config = objectMapper.convertValue(Map.of( + "enabled", true, + "botToken", "xoxb-test", + "appToken", "xapp-test", + "replyInThread", true, + "allowedUserIds", List.of("U111")), SlackPluginConfig.class); + when(configService.getConfig()).thenReturn(config); adapter.start(); ArgumentCaptor> captor = ArgumentCaptor.forClass(Consumer.class); @@ -119,7 +127,7 @@ void shouldIgnoreUnauthorizedUsers() { true, false)); - verify(eventPublisher, never()).publishEvent(any()); + verify(eventPublisher).publishEvent(any(AgentLoop.InboundMessageEvent.class)); } @Test From 720ad2810691ae856b4fd4eb3039c3e3f05d257b Mon Sep 17 00:00:00 2001 From: Alex Kuleshov Date: Sun, 22 Mar 2026 13:24:08 -0400 Subject: [PATCH 2/4] chore(release): golemcore/slack v1.0.4 --- .../2026-03-22-slack-remove-allowlists.md | 20 +++++++++++++++++-- ...26-03-22-slack-remove-allowlists-design.md | 4 +++- golemcore/slack/plugin.yaml | 2 +- golemcore/slack/pom.xml | 2 +- .../SlackPluginSettingsContributorTest.java | 1 - registry/golemcore/slack/index.yaml | 3 ++- registry/golemcore/slack/versions/1.0.4.yaml | 13 ++++++++++++ 7 files changed, 38 insertions(+), 7 deletions(-) create mode 100644 registry/golemcore/slack/versions/1.0.4.yaml diff --git a/docs/superpowers/plans/2026-03-22-slack-remove-allowlists.md b/docs/superpowers/plans/2026-03-22-slack-remove-allowlists.md index 05acd4f..679529b 100644 --- a/docs/superpowers/plans/2026-03-22-slack-remove-allowlists.md +++ b/docs/superpowers/plans/2026-03-22-slack-remove-allowlists.md @@ -2,7 +2,7 @@ > **For agentic workers:** REQUIRED: Use superpowers:subagent-driven-development (if subagents available) or superpowers:executing-plans to implement this plan. Steps use checkbox (`- [ ]`) syntax for tracking. -**Goal:** Remove Slack user/channel allowlists from config, UI, and runtime filtering while keeping legacy stored keys harmless. +**Goal:** Remove Slack user/channel allowlists from config, UI, and runtime filtering while keeping legacy stored keys harmless, and publish the fix as Slack plugin version `1.0.4`. **Architecture:** Keep the persisted-config compatibility boundary in `SlackPluginConfig` via unknown-field tolerance, simplify the settings surface, and remove runtime allowlist checks from `SlackAdapter`. The change is intentionally narrow and does not alter mention or thread routing semantics. @@ -64,7 +64,23 @@ Delete the inbound/action filter methods in `SlackAdapter` and keep only the thr Run: `mvn -B -ntp -f pom.xml -pl :golemcore-slack-plugin -Dtest=SlackPluginConfigServiceTest,SlackPluginSettingsContributorTest,SlackAdapterTest test` -### Task 3: Verify And Prepare PR +### Task 3: Bump Slack Release Metadata To 1.0.4 + +**Files:** +- Modify: `golemcore/slack/pom.xml` +- Modify: `golemcore/slack/plugin.yaml` +- Modify: `registry/golemcore/slack/index.yaml` +- Create: `registry/golemcore/slack/versions/1.0.4.yaml` + +- [ ] **Step 1: Run the repository release helper with an explicit Slack version** + +Run: `python3 scripts/plugins_repo.py release --plugin golemcore/slack --version-override 1.0.4` + +- [ ] **Step 2: Verify the generated metadata references 1.0.4** + +Check that the Slack module version, plugin manifest version, registry latest pointer, and new registry version file are all aligned on `1.0.4`. + +### Task 4: Verify And Prepare PR **Files:** - Verify only diff --git a/docs/superpowers/specs/2026-03-22-slack-remove-allowlists-design.md b/docs/superpowers/specs/2026-03-22-slack-remove-allowlists-design.md index ade3844..f3be918 100644 --- a/docs/superpowers/specs/2026-03-22-slack-remove-allowlists-design.md +++ b/docs/superpowers/specs/2026-03-22-slack-remove-allowlists-design.md @@ -2,7 +2,7 @@ ## Goal -Remove `allowedUserIds` and `allowedChannelIds` from the Slack plugin's public configuration and runtime behavior so Slack message intake depends only on Slack event type, bot installation, and existing thread/session rules. +Remove `allowedUserIds` and `allowedChannelIds` from the Slack plugin's public configuration and runtime behavior so Slack message intake depends only on Slack event type, bot installation, and existing thread/session rules, and ship that change as Slack plugin version `1.0.4`. ## Scope @@ -10,6 +10,7 @@ Remove `allowedUserIds` and `allowedChannelIds` from the Slack plugin's public c - Remove both fields from the Slack settings UI and save flow. - Remove inbound user/channel filtering from the Slack adapter. - Continue tolerating legacy persisted config keys silently. +- Publish the change as patch release `1.0.4` with refreshed registry metadata and checksum. ## Non-Goals @@ -32,3 +33,4 @@ Inbound Slack handling will no longer call allowlist-based user/channel filters. - Update config-service tests to verify legacy allowlist keys are ignored on read and are not persisted on save. - Update settings-contributor tests to verify the section no longer exposes allowlist fields and save keeps only supported values. - Update adapter tests to verify inbound messages are not dropped because of prior allowlist configuration. +- Verify the release metadata points to `1.0.4` and matches the locally built artifact checksum. diff --git a/golemcore/slack/plugin.yaml b/golemcore/slack/plugin.yaml index 8797145..678865c 100644 --- a/golemcore/slack/plugin.yaml +++ b/golemcore/slack/plugin.yaml @@ -1,7 +1,7 @@ id: golemcore/slack provider: golemcore name: slack -version: 1.0.3 +version: 1.0.4 pluginApiVersion: 1 engineVersion: ">=0.0.0 <1.0.0" entrypoint: me.golemcore.plugins.golemcore.slack.SlackPluginBootstrap diff --git a/golemcore/slack/pom.xml b/golemcore/slack/pom.xml index ab9f7f2..69b20ec 100644 --- a/golemcore/slack/pom.xml +++ b/golemcore/slack/pom.xml @@ -9,7 +9,7 @@ ../../pom.xml - 1.0.3 + 1.0.4 golemcore-slack-plugin golemcore/slack Slack Socket Mode channel plugin with interactive approvals for GolemCore diff --git a/golemcore/slack/src/test/java/me/golemcore/plugins/golemcore/slack/SlackPluginSettingsContributorTest.java b/golemcore/slack/src/test/java/me/golemcore/plugins/golemcore/slack/SlackPluginSettingsContributorTest.java index e2108bc..caeed53 100644 --- a/golemcore/slack/src/test/java/me/golemcore/plugins/golemcore/slack/SlackPluginSettingsContributorTest.java +++ b/golemcore/slack/src/test/java/me/golemcore/plugins/golemcore/slack/SlackPluginSettingsContributorTest.java @@ -8,7 +8,6 @@ import org.mockito.ArgumentCaptor; import org.springframework.context.ApplicationEventPublisher; -import java.util.List; import java.util.Map; import static org.junit.jupiter.api.Assertions.assertEquals; diff --git a/registry/golemcore/slack/index.yaml b/registry/golemcore/slack/index.yaml index 0bfcc7c..7016a14 100644 --- a/registry/golemcore/slack/index.yaml +++ b/registry/golemcore/slack/index.yaml @@ -1,10 +1,11 @@ id: golemcore/slack owner: golemcore name: slack -latest: 1.0.3 +latest: 1.0.4 versions: - 1.0.0 - 1.0.1 - 1.0.2 - 1.0.3 + - 1.0.4 source: "https://github.com/alexk-dev/golemcore-plugins/tree/main/golemcore/slack" diff --git a/registry/golemcore/slack/versions/1.0.4.yaml b/registry/golemcore/slack/versions/1.0.4.yaml new file mode 100644 index 0000000..fee0fcc --- /dev/null +++ b/registry/golemcore/slack/versions/1.0.4.yaml @@ -0,0 +1,13 @@ +id: golemcore/slack +version: 1.0.4 +pluginApiVersion: 1 +engineVersion: ">=0.0.0 <1.0.0" +artifactUrl: "dist/golemcore/slack/1.0.4/golemcore-slack-plugin-1.0.4.jar" +checksumSha256: "4ddab497d829c7e0e20acf1d8a156fd627e946e6c96891cec5614636a7838cc0" +publishedAt: "2026-03-22T17:22:33Z" +sourceCommit: "57967fbeb9aee29b37e3cc60367c122611a84641" +entrypoint: me.golemcore.plugins.golemcore.slack.SlackPluginBootstrap +sourceUrl: "https://github.com/alexk-dev/golemcore-plugins/tree/main/golemcore/slack" +license: "Apache-2.0" +maintainers: + - alexk-dev From a9276d2e2574e4f9275ea38e800677ca09262310 Mon Sep 17 00:00:00 2001 From: Alex Kuleshov Date: Sun, 22 Mar 2026 13:33:01 -0400 Subject: [PATCH 3/4] fix(repo): align slack 1.0.4 checksum with ci --- registry/golemcore/slack/versions/1.0.4.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/registry/golemcore/slack/versions/1.0.4.yaml b/registry/golemcore/slack/versions/1.0.4.yaml index fee0fcc..e4b59f7 100644 --- a/registry/golemcore/slack/versions/1.0.4.yaml +++ b/registry/golemcore/slack/versions/1.0.4.yaml @@ -3,7 +3,7 @@ version: 1.0.4 pluginApiVersion: 1 engineVersion: ">=0.0.0 <1.0.0" artifactUrl: "dist/golemcore/slack/1.0.4/golemcore-slack-plugin-1.0.4.jar" -checksumSha256: "4ddab497d829c7e0e20acf1d8a156fd627e946e6c96891cec5614636a7838cc0" +checksumSha256: "bd1d03fb07a9c8173a087acd64a2764fa5a3b903e7fcbbbc4468a830e84ee72f" publishedAt: "2026-03-22T17:22:33Z" sourceCommit: "57967fbeb9aee29b37e3cc60367c122611a84641" entrypoint: me.golemcore.plugins.golemcore.slack.SlackPluginBootstrap From b6d04c3fc76b17df7771d731a6d9c4a243443ea0 Mon Sep 17 00:00:00 2001 From: Alex Kuleshov Date: Sun, 22 Mar 2026 13:38:32 -0400 Subject: [PATCH 4/4] chore(pr): drop superpowers docs from slack pr --- .../2026-03-22-slack-remove-allowlists.md | 102 ------------------ ...26-03-22-slack-remove-allowlists-design.md | 36 ------- 2 files changed, 138 deletions(-) delete mode 100644 docs/superpowers/plans/2026-03-22-slack-remove-allowlists.md delete mode 100644 docs/superpowers/specs/2026-03-22-slack-remove-allowlists-design.md diff --git a/docs/superpowers/plans/2026-03-22-slack-remove-allowlists.md b/docs/superpowers/plans/2026-03-22-slack-remove-allowlists.md deleted file mode 100644 index 679529b..0000000 --- a/docs/superpowers/plans/2026-03-22-slack-remove-allowlists.md +++ /dev/null @@ -1,102 +0,0 @@ -# Slack Remove Allowlists Implementation Plan - -> **For agentic workers:** REQUIRED: Use superpowers:subagent-driven-development (if subagents available) or superpowers:executing-plans to implement this plan. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Remove Slack user/channel allowlists from config, UI, and runtime filtering while keeping legacy stored keys harmless, and publish the fix as Slack plugin version `1.0.4`. - -**Architecture:** Keep the persisted-config compatibility boundary in `SlackPluginConfig` via unknown-field tolerance, simplify the settings surface, and remove runtime allowlist checks from `SlackAdapter`. The change is intentionally narrow and does not alter mention or thread routing semantics. - -**Tech Stack:** Java 25, Spring, JUnit 5, Mockito, Maven - ---- - -### Task 1: Update Tests For Removed Allowlists - -**Files:** -- Modify: `golemcore/slack/src/test/java/me/golemcore/plugins/golemcore/slack/SlackPluginConfigServiceTest.java` -- Modify: `golemcore/slack/src/test/java/me/golemcore/plugins/golemcore/slack/SlackPluginSettingsContributorTest.java` -- Modify: `golemcore/slack/src/test/java/me/golemcore/plugins/golemcore/slack/adapter/inbound/slack/SlackAdapterTest.java` - -- [ ] **Step 1: Write failing assertions for config-service behavior** - -Remove expectations that `allowedUserIds` and `allowedChannelIds` exist in the runtime config or saved payload, while keeping the legacy input keys in the mocked stored config. - -- [ ] **Step 2: Run the targeted config-service test to verify it fails** - -Run: `mvn -B -ntp -f pom.xml -pl :golemcore-slack-plugin -Dtest=SlackPluginConfigServiceTest test` - -- [ ] **Step 3: Write failing assertions for settings section exposure/save** - -Update the settings contributor tests so the section no longer exposes the allowlist values and save payloads no longer accept or persist them. - -- [ ] **Step 4: Run the targeted settings-contributor test to verify it fails** - -Run: `mvn -B -ntp -f pom.xml -pl :golemcore-slack-plugin -Dtest=SlackPluginSettingsContributorTest test` - -- [ ] **Step 5: Write failing assertions for inbound message handling** - -Update the adapter tests so prior allowlist-style config does not cause inbound messages to be ignored. - -- [ ] **Step 6: Run the targeted adapter test to verify it fails** - -Run: `mvn -B -ntp -f pom.xml -pl :golemcore-slack-plugin -Dtest=SlackAdapterTest test` - -### Task 2: Remove Allowlists From Slack Runtime And Settings - -**Files:** -- Modify: `golemcore/slack/src/main/java/me/golemcore/plugins/golemcore/slack/SlackPluginConfig.java` -- Modify: `golemcore/slack/src/main/java/me/golemcore/plugins/golemcore/slack/SlackPluginSettingsContributor.java` -- Modify: `golemcore/slack/src/main/java/me/golemcore/plugins/golemcore/slack/adapter/inbound/slack/SlackAdapter.java` - -- [ ] **Step 1: Remove allowlist fields and normalization from the config model** - -Delete `allowedUserIds` and `allowedChannelIds` from `SlackPluginConfig` and keep normalization focused on supported fields. - -- [ ] **Step 2: Remove allowlist fields from the settings section and save flow** - -Delete the UI fields, values, and save-path parsing for the two legacy keys. - -- [ ] **Step 3: Remove runtime user/channel allowlist checks** - -Delete the inbound/action filter methods in `SlackAdapter` and keep only the thread/session gating logic. - -- [ ] **Step 4: Run the targeted Slack test class to verify the implementation passes** - -Run: `mvn -B -ntp -f pom.xml -pl :golemcore-slack-plugin -Dtest=SlackPluginConfigServiceTest,SlackPluginSettingsContributorTest,SlackAdapterTest test` - -### Task 3: Bump Slack Release Metadata To 1.0.4 - -**Files:** -- Modify: `golemcore/slack/pom.xml` -- Modify: `golemcore/slack/plugin.yaml` -- Modify: `registry/golemcore/slack/index.yaml` -- Create: `registry/golemcore/slack/versions/1.0.4.yaml` - -- [ ] **Step 1: Run the repository release helper with an explicit Slack version** - -Run: `python3 scripts/plugins_repo.py release --plugin golemcore/slack --version-override 1.0.4` - -- [ ] **Step 2: Verify the generated metadata references 1.0.4** - -Check that the Slack module version, plugin manifest version, registry latest pointer, and new registry version file are all aligned on `1.0.4`. - -### Task 4: Verify And Prepare PR - -**Files:** -- Verify only - -- [ ] **Step 1: Run the Slack module package build** - -Run: `mvn -B -ntp -f pom.xml -pl :golemcore-slack-plugin -am package` - -- [ ] **Step 2: Run repository validation** - -Run: `python3 scripts/plugins_repo.py validate` - -- [ ] **Step 3: Commit the change** - -Use: `git add ... && git commit -m "fix(slack): remove legacy allowlists"` - -- [ ] **Step 4: Push branch and open PR** - -Use: `git push -u origin fix/slack-remove-allowlists` then create the PR against `main`. diff --git a/docs/superpowers/specs/2026-03-22-slack-remove-allowlists-design.md b/docs/superpowers/specs/2026-03-22-slack-remove-allowlists-design.md deleted file mode 100644 index f3be918..0000000 --- a/docs/superpowers/specs/2026-03-22-slack-remove-allowlists-design.md +++ /dev/null @@ -1,36 +0,0 @@ -# Slack Remove Allowlists Design - -## Goal - -Remove `allowedUserIds` and `allowedChannelIds` from the Slack plugin's public configuration and runtime behavior so Slack message intake depends only on Slack event type, bot installation, and existing thread/session rules, and ship that change as Slack plugin version `1.0.4`. - -## Scope - -- Remove both allowlist fields from the Slack config model. -- Remove both fields from the Slack settings UI and save flow. -- Remove inbound user/channel filtering from the Slack adapter. -- Continue tolerating legacy persisted config keys silently. -- Publish the change as patch release `1.0.4` with refreshed registry metadata and checksum. - -## Non-Goals - -- No storage migration job. -- No change to mention-only behavior for top-level channel messages. -- No change to thread/session routing. - -## Design - -The plugin will keep `@JsonIgnoreProperties(ignoreUnknown = true)` on the config model, which lets older saved configs still deserialize cleanly even after the Java fields are removed. The settings contributor will stop exposing and saving the legacy keys, so any subsequent save naturally rewrites the stored config without them. - -Inbound Slack handling will no longer call allowlist-based user/channel filters. The adapter will continue to enforce the existing event-shape rules: - -- DMs are accepted. -- `app_mention` events are accepted. -- thread replies are accepted only when they belong to an active conversation. - -## Tests - -- Update config-service tests to verify legacy allowlist keys are ignored on read and are not persisted on save. -- Update settings-contributor tests to verify the section no longer exposes allowlist fields and save keeps only supported values. -- Update adapter tests to verify inbound messages are not dropped because of prior allowlist configuration. -- Verify the release metadata points to `1.0.4` and matches the locally built artifact checksum.