fix(slack-gateway): preserve ACP reply whitespace

[onutc]

TL;DR

This fixes Slack reply corruption caused by trimming and rejoining ACP text chunks at the gateway boundary. The ACP text extraction logic is now shared between the API and Slack gateway, and the build/test wiring was updated so the shared module is exercised in container builds and CI.

Summary

• preserve ACP chunk whitespace and newlines in the Slack gateway reply path
• move ACP text extraction and chunk joining into a shared lossless helper used by the API and Slack gateway
• stop trimming the final Slack chat.postMessage text payload
• add regression coverage for chunk-boundary whitespace and final Slack payload integrity
• stage the shared acptext module in API and Slack gateway image builds
• run the Go workflow for acptext changes and add the new module to the Go test matrix
• document the ACP reply text integrity invariant in the existing implementation plan

Review Focus

• acptext semantics for ACP text, content, and resource payloads
• Slack gateway behavior when ACP returns whitespace-sensitive multiline replies
• Dockerfile staging for local replace targets and CI coverage for shared-module changes

Test Plan

• go test ./... in acptext
• go test ./... in api
• go test ./... in integrations/slack-gateway
• docker build -f api/Dockerfile . -t spritz-api-whitespace-check
• docker build -f integrations/slack-gateway/Dockerfile . -t spritz-slack-gateway-whitespace-check
• ruby -e 'require "yaml"; YAML.load_file(".github/workflows/go-tests.yml"); puts "ok"'\n- [x] git diff --check\n- [x] npx -y @simpledoc/simpledoc check\n

[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 implementation report for #195

Completed:

• moved the shared ACP text helper to integrations/acptext so both api and integrations/slack-gateway use one lossless text extraction/join path
• preserved Slack reply whitespace end-to-end by removing destructive trimming from ACP chunk assembly and chat.postMessage payloads
• added regression coverage for chunk-boundary whitespace/newlines and empty-resource-text URI fallback in Go, UI, and e2e helpers
• added Docker/CI coverage for the shared-module image builds and documented the required repo-root Docker build context in the affected Dockerfiles
• updated the existing Slack gateway implementation plan doc with the ACP reply text integrity contract

Validation run locally:

• go test ./... in integrations/acptext: pass
• go test ./... in api: pass
• go test ./... in integrations/slack-gateway: pass
• node --test e2e/acp-smoke-lib.test.mjs: pass
• pnpm --dir ui test -- src/lib/acp-client.test.ts: pass
• docker build -f api/Dockerfile . -t spritz-api-whitespace-check: pass
• docker build -f integrations/slack-gateway/Dockerfile . -t spritz-slack-gateway-whitespace-check: pass
• ruby -e 'require "yaml"; YAML.load_file(".github/workflows/go-tests.yml"); puts "ok"': pass
• git diff --check: pass
• npx -y @simpledoc/simpledoc check: pass

Review / CI status:

• local codex review --base main: no correctness findings on the final branch head
• fresh PR AI review prompt posted for head b34ae1cb1b9de6a20b4c9c1dff1bcb937ea0b7d4; no new Codex/Claude issue or inline comments arrived during the watch window after 2026-04-01T17:56:58Z
• tcx pr can-merge --pr 195 --repo textcortex/spritz: Result: CAN MERGE (checks)

[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 fixes a bug where the Slack gateway was corrupting ACP assistant replies by trimming whitespace and newlines at chunk boundaries. It extracts shared ACP text processing logic into a new acptext module used by both the API and Slack gateway, ensuring consistent lossless handling of whitespace-sensitive content. The fix includes comprehensive test coverage and CI/CD updates to validate the shared module across container builds.

Analysis Details

Component Classification: This PR affects multiple components (Slack gateway, API, shared utilities) without a single dominant area. The changes are cross-cutting infrastructure improvements for ACP text handling, making OTHER the appropriate classification.

Severity Justification: This is a P2 (Medium) bug fix. The issue causes silent data corruption in Slack replies when ACP returns multiline content with whitespace-sensitive formatting, but it has a workaround (manual message correction) and doesn't cause service outages or complete data loss.

Eligibility Notes: Issue: True - PR explicitly fixes a bug (reply corruption from whitespace trimming). Fix Implementation: True - code changes directly address the problem by preserving chunk boundaries and using shared extraction logic. PR Linked: True - detailed description with TL;DR, summary, review focus, and test plan. Tests: True - includes unit tests in acptext_test.go, gateway_test.go, and e2e tests. Tests Required: True - this is a business logic bug fix affecting data integrity that requires regression tests to prevent recurrence.

---

Analyzed by GitRank 🤖