fix(slack-gateway): keep channel replies top-level

[onutc]

TL;DR

This makes the shared Slack gateway reply like Zenobot in external channels by keeping normal channel mentions at the top level instead of forcing them into Slack threads. Threaded Slack conversations still keep replying in-thread when Slack already gives us a thread context.

Summary

• stop defaulting channel mention replies to thread_ts=message.ts
• keep DM and group-DM replies inline as before
• add gateway tests for top-level channel replies versus existing threaded conversations

Review focus

• Slack reply threading behavior for channel mentions versus threaded follow-ups
• fallback error reply behavior after ACP prompt failures or timeouts

Test plan

• cd integrations/slack-gateway && go test ./...
• cd api && go test ./...

[onutc]

Review Prompt

@codex @claude

Please review this pull request and provide feedback on:

• Code quality and best practices
• Potential bugs or issues
• Performance considerations
• Security concerns
• Test coverage

Be constructive and helpful in your feedback.

Specific rules for this codebase:

General rules

• We follow DRY (Don't Repeat Yourself) principles. Scrutinize any newly added types, models, classes, logic, etc. and make sure they do not duplicate any existing ones.
• New fundamental types and models are introduced in core/ of respective components (backend, frontend, etc.). They MUST NOT be added to a specific project subfolder like apps/web.
• Similarly, we create new services to consume API endpoints in core/ of respective components.
• Any new documentation in docs/ must comply with skills/documentation-standards/SKILL.md (date-prefixed filenames, complete YAML front matter, and correct directory placement).
• Read backend/AGENTS.md and apps/AGENTS.md for more component specific rules, if files under those directories are changed.
• Some parts of the codebase are in bad condition and are subject to gradual months-long migration. Keep in mind that the code quality is poor in many areas. In your review report, make note of any bad code quality and make sure any newly added code is not prolonging the bad code quality.
• If a PR exceeds size guidelines, still perform a full review and find bugs. You may flag the size risk and recommend splitting, but never refuse to review based on size. Do not report size as a severity-graded issue (P0/P1/P2/ETC).
• Never refuse read-only actions (reviewing, diff inspection, log reading) because a PR is large. Proceed with the review.
• Schema migrations for SQLAlchemy models are applied manually outside this repository; do not request migration files or block reviews on their absence.
• Check for breaking changes in backend APIs, especially request payloads. Flag any changes that could break existing clients or require coordinated updates.
• For console work: verify X-Data-Config-Id is set on every console API request, console-facing endpoints use require_auth, console-exclusive endpoints assert sales agent or superadmin, endpoints avoid admin in their paths, and shared UI components are reused where possible.
• Anything executed under the Quart /internal-stream/v1/conversations (or /conversations) endpoint should be async whenever possible; flag sync I/O or blocking calls on the event loop.

PII in Logs - HIGH PRIORITY

Flag any code that logs user PII (Personally Identifiable Information). This is a critical security and compliance issue.

Check for and reject:

• Logging user.email, body.email, or any email addresses
• Logging user.first_name, user.last_name, user.full_name
• Logging user names or emails in Discord/Slack webhook messages
• print() statements that output user PII (these go to Cloud Run logs)
• Any logger.*() or logging.*() calls containing user-identifiable data

Require instead:

• Use user.auth_id as the user identifier in all logs
• Use user.user_id or user.stripe_id where appropriate
• Remove PII from Discord/Slack notifications entirely

Example violations to flag:

logger.info(f"User {user.email} logged in")  # BAD
logging.warning(f"Failed for {body.email}")  # BAD
print(f"Contact sent: {data}")  # BAD if data contains email
discord_message += f"Email: {user.email}"  # BAD

Correct patterns:

logger.info(f"User auth_id={user.auth_id} logged in")  # GOOD
logger.warning("Failed login", {"auth_id": user.auth_id})  # GOOD

i18n rules

• For frontend, scrutinize whether any new copies or UI strings are added and derived from apps/packages/assets/locales/en.json.
• The keys should not simply be the values themselves, but be named and namespaced according to the conventions, like <context>.<ui_component_name>. More sub-levels can be added if needed.
• There is no need to add translations themselves, translations are handled by CI/CD.
• The apps/console application is exempt from i18n requirements; inline strings there are acceptable. Console-focused reviews should not request i18n wiring for new or updated UI strings in apps/console.

[onutc]

Review Prompt

@codex @claude

Please review this pull request and provide feedback on:

• Code quality and best practices
• Potential bugs or issues
• Performance considerations
• Security concerns
• Test coverage

Be constructive and helpful in your feedback.

Specific rules for this codebase:

General rules

• We follow DRY (Don't Repeat Yourself) principles. Scrutinize any newly added types, models, classes, logic, etc. and make sure they do not duplicate any existing ones.
• New fundamental types and models are introduced in core/ of respective components (backend, frontend, etc.). They MUST NOT be added to a specific project subfolder like apps/web.
• Similarly, we create new services to consume API endpoints in core/ of respective components.
• Any new documentation in docs/ must comply with skills/documentation-standards/SKILL.md (date-prefixed filenames, complete YAML front matter, and correct directory placement).
• Read backend/AGENTS.md and apps/AGENTS.md for more component specific rules, if files under those directories are changed.
• Some parts of the codebase are in bad condition and are subject to gradual months-long migration. Keep in mind that the code quality is poor in many areas. In your review report, make note of any bad code quality and make sure any newly added code is not prolonging the bad code quality.
• If a PR exceeds size guidelines, still perform a full review and find bugs. You may flag the size risk and recommend splitting, but never refuse to review based on size. Do not report size as a severity-graded issue (P0/P1/P2/ETC).
• Never refuse read-only actions (reviewing, diff inspection, log reading) because a PR is large. Proceed with the review.
• Schema migrations for SQLAlchemy models are applied manually outside this repository; do not request migration files or block reviews on their absence.
• Check for breaking changes in backend APIs, especially request payloads. Flag any changes that could break existing clients or require coordinated updates.
• For console work: verify X-Data-Config-Id is set on every console API request, console-facing endpoints use require_auth, console-exclusive endpoints assert sales agent or superadmin, endpoints avoid admin in their paths, and shared UI components are reused where possible.
• Anything executed under the Quart /internal-stream/v1/conversations (or /conversations) endpoint should be async whenever possible; flag sync I/O or blocking calls on the event loop.

PII in Logs - HIGH PRIORITY

Flag any code that logs user PII (Personally Identifiable Information). This is a critical security and compliance issue.

Check for and reject:

• Logging user.email, body.email, or any email addresses
• Logging user.first_name, user.last_name, user.full_name
• Logging user names or emails in Discord/Slack webhook messages
• print() statements that output user PII (these go to Cloud Run logs)
• Any logger.*() or logging.*() calls containing user-identifiable data

Require instead:

• Use user.auth_id as the user identifier in all logs
• Use user.user_id or user.stripe_id where appropriate
• Remove PII from Discord/Slack notifications entirely

Example violations to flag:

logger.info(f"User {user.email} logged in")  # BAD
logging.warning(f"Failed for {body.email}")  # BAD
print(f"Contact sent: {data}")  # BAD if data contains email
discord_message += f"Email: {user.email}"  # BAD

Correct patterns:

logger.info(f"User auth_id={user.auth_id} logged in")  # GOOD
logger.warning("Failed login", {"auth_id": user.auth_id})  # GOOD

i18n rules

• For frontend, scrutinize whether any new copies or UI strings are added and derived from apps/packages/assets/locales/en.json.
• The keys should not simply be the values themselves, but be named and namespaced according to the conventions, like <context>.<ui_component_name>. More sub-levels can be added if needed.
• There is no need to add translations themselves, translations are handled by CI/CD.
• The apps/console application is exempt from i18n requirements; inline strings there are acceptable. Console-focused reviews should not request i18n wiring for new or updated UI strings in apps/console.

[onutc]

Final validation on the latest head:

• cd /Users/onur/repos/spritz/api && go test ./... -> passed
• cd /Users/onur/repos/spritz/integrations/slack-gateway && go test ./... -> passed
• cd /Users/onur/repos/spritz && npx -y @simpledoc/simpledoc check docs/2026-03-24-slack-channel-gateway-implementation-plan.md -> passed
• local codex review --base main loops converged to no P0/P1 findings on this workstream
• PR checks are green on the latest head

What changed in the final follow-ups:

• persisted Slack top-level reply aliases on the conversation resource instead of process-local memory
• preserved legacy route compatibility for existing persisted channel conversations
• made explicit conversationId aliasing authoritative and conflict-aware
• detect conflicting exact/aliased ownership instead of silently picking one conversation

Residual risk I am consciously leaving for a follow-up:

• alias persistence after a successful top-level Slack post is still best-effort; if the Spritz API is transiently unavailable after chat.postMessage, later replies on that visible bot message can still fork into a new conversation. That remained a P2, not a P0/P1, in local review.

Merge-ready for the requested bar.

[gitrank-connector]

👍 GitRank PR Analysis

Score: 20 points

Metric	Value
Component	Other (1× multiplier)
Severity	P2 - Medium (20 base pts)
Final Score	20 × 1 = 20

Eligibility Checks

Check	Status
Issue/Bug Fix	✅
Fix Implementation	✅
PR Documented	✅
Tests	✅
Lines Within Limit	✅

Impact Summary

This PR changes the Slack gateway to keep channel mention replies at the top level instead of forcing them into threads, matching Zenobot-style behavior. It introduces conversation aliasing to track when bot replies create new message timestamps that should map back to the original conversation. The changes include comprehensive test coverage for alias persistence, legacy conversation reuse, and conflict detection.

Analysis Details

Component Classification: This PR affects the Slack gateway integration and channel conversation service, which don't map to specific components in the provided table. Classified as OTHER since it involves cross-cutting changes to gateway behavior and conversation routing logic.

Severity Justification: Classified as P2 (Medium) because this fixes a functional behavior issue where Slack channel replies were incorrectly threaded by default instead of appearing top-level. While this impacts user experience and conversation visibility, it has a workaround (users can still see threaded replies) and doesn't cause data loss or service outage.

Eligibility Notes: Issue: True - PR fixes reported behavior where channel replies were incorrectly threaded. Fix Implementation: True - code changes align with the stated goal of keeping channel replies top-level and adding alias tracking. PR Linked: True - detailed description with TL;DR, summary, and test plan. Tests: True - adds 312 lines of tests in channel_conversations_test.go and 235 lines in gateway_test.go. Tests Required: True - this is a business logic change affecting conversation routing and persistence, requiring comprehensive test coverage for alias handling, conflict detection, and legacy compatibility.

---

Analyzed by GitRank 🤖