Refactor Slack App Home module#690
Merged
Merged
Conversation
📝 WalkthroughWalkthroughModularizes Slack App Home into typed blocks/constants, view builders, modal metadata, interaction routing, publisher, and KV-backed user preference resolution; removes prior inline handling from index.ts and updates tests. ChangesSlack App Home UI and Interactions
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Terraform Validation Results
Pushed by: @ColeMurray, Action: |
![open-inspect[bot]](https://avatars.githubusercontent.com/u/2492022?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Summary
PR #690, "Refactor Slack App Home module" by @ColeMurray, moves Slack App Home publishing, interactions, modals, metadata, view construction, model lookup, and user preference handling into focused modules. I reviewed the 13 changed files (+1349/-939) and the refactor preserves the existing route behavior while improving separation of concerns.
Critical Issues
None found.
Suggestions
None blocking. The extracted app-home/ modules are cohesive and keep App Home-specific behavior out of the generic Slack interaction handler.
Nitpicks
None.
Positive Feedback
- The App Home interaction dispatch table makes it clearer which actions are handled inline versus scheduled in the background.
- Preference resolution is centralized in
user-preferences.ts, reducing duplicated model/reasoning/branch normalization logic. - The repo override block-limit coverage was preserved at the view-construction level, which is a cleaner unit boundary for that behavior.
Questions
None.
Validation
git diff --check origin/main...refs/tmp/pr-690passed.- I could not run the Slack bot test/typecheck commands in this environment because dev dependencies are not installed, so
vitestandtscwere unavailable.
Verdict
Approve.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/slack-bot/src/app-home/view.ts`:
- Around line 49-54: toSelectOption is passing model.label verbatim which can
exceed Slack's 75-character static_select text limit and break App Home render;
update toSelectOption (which returns SlackSelectOption from ModelOption) to
truncate model.label to 75 characters and append an ellipsis if truncated,
matching the truncation behavior used by buildRepoBranchSelectOptions so all
select option text fields conform to Slack's limit and avoid rejection.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fba72d23-f874-44e7-880b-412420993fed
📒 Files selected for processing (13)
packages/slack-bot/src/app-home.test.tspackages/slack-bot/src/app-home/constants.tspackages/slack-bot/src/app-home/index.tspackages/slack-bot/src/app-home/interactions.tspackages/slack-bot/src/app-home/metadata.tspackages/slack-bot/src/app-home/modals.tspackages/slack-bot/src/app-home/models.tspackages/slack-bot/src/app-home/publisher.tspackages/slack-bot/src/app-home/slack-types.tspackages/slack-bot/src/app-home/view.tspackages/slack-bot/src/index.test.tspackages/slack-bot/src/index.tspackages/slack-bot/src/user-preferences.ts
Terraform Validation Results
Pushed by: @ColeMurray, Action: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/slack-bot/src/user-preferences.test.ts`:
- Around line 31-50: The test name is misleading because
mergeUserPreferencesPatch currently clears reasoning whenever the patch includes
a model field, even if it's the same; update mergeUserPreferencesPatch (and any
callers like updateUserPreferences) to only set reasoningEffort to undefined
when patch.model is present AND patch.model !== existingPrefs.model so
normalization will only apply getDefaultReasoningEffort(model) when the model
actually changed; alternatively if you prefer test-side fix, rename the test to
indicate it triggers on any model patch (e.g., "resets reasoning when model is
patched") and keep current logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1f00dcc3-2a2d-4690-98fb-2dc4de5b974b
📒 Files selected for processing (6)
packages/slack-bot/src/app-home.test.tspackages/slack-bot/src/app-home/interactions.tspackages/slack-bot/src/app-home/view.tspackages/slack-bot/src/index.test.tspackages/slack-bot/src/user-preferences.test.tspackages/slack-bot/src/user-preferences.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/slack-bot/src/app-home.test.ts
- packages/slack-bot/src/user-preferences.ts
- packages/slack-bot/src/app-home/interactions.ts
hreiten
added a commit
to watchdog-no/background-agents
that referenced
this pull request
May 31, 2026
* Allow Slack sessions to use OpenCode titles (ColeMurray#685) ## Summary - Stop pre-filling Slack-created session titles from the Slack message or repo fallback - Leave Slack session titles unset so OpenCode-generated title events can populate them - Add a regression assertion for the Slack session creation payload ## Validation - npm run build -w @open-inspect/shared - npm test -w @open-inspect/slack-bot - npm run typecheck -w @open-inspect/slack-bot - npm run lint -w @open-inspect/slack-bot <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Removed title field from session creation requests to the control plane. * **Tests** * Updated tests to verify title field is not included in session creation requests. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/685?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> * Add provider identity resolution API (ColeMurray#686) ## Summary - Add a shared canonical user ID validator for 32-character lowercase hex D1 user IDs. - Add an internal HMAC-authenticated `PUT /provider-identities/:provider/:providerUserId` control-plane route that upserts provider identity metadata through `UserStore.resolveOrCreateUser` and returns the canonical `userId`. - Add browser-facing `GET /api/me`, deriving GitHub identity from the server-side NextAuth session and returning only the canonical user ID. ## Validation - `npm run build -w @open-inspect/shared` - `npm test -w @open-inspect/shared -- user-id` - `npm run typecheck -w @open-inspect/shared` - `npm run lint -w @open-inspect/shared -- --quiet` - `npm test -w @open-inspect/control-plane -- routes/provider-identities.test.ts router.provider-identities.test.ts` - `npm run typecheck -w @open-inspect/control-plane` - `npm run lint -w @open-inspect/control-plane -- --quiet` - `npm test -w @open-inspect/web -- app/api/me/route.test.ts` - `npm run typecheck -w @open-inspect/web` - `npm run lint -w @open-inspect/web -- --quiet` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * New API to return the authenticated user's GitHub-derived identity and canonical user ID. * New endpoints to upsert GitHub provider identities and return a canonical user ID. * Shared validator ensuring canonical 32-character lowercase hex user IDs. * **Bug Fixes** * Improved input validation and explicit error responses for invalid provider-user IDs and malformed requests. * Better error handling when identity resolution returns unexpected values. * **Tests** * Added integration and unit tests covering success, validation failures, and error paths. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/686?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> * Filter sessions by creator (ColeMurray#688) ## Summary - Add `createdBy` session-list filtering for canonical user IDs in the control plane and web BFF - Add a sidebar All/Mine filter that resolves the current canonical user via `/api/me` - Revalidate all session-list SWR cache variants and add a D1 index for creator-filtered recency queries ## Validation - `npm test -w @open-inspect/control-plane -- src/db/session-index.test.ts` - `npm run test:integration -w @open-inspect/control-plane -- test/integration/auth.test.ts` - `npm test -w @open-inspect/web -- src/lib/session-list.test.ts src/lib/control-plane-query.test.ts src/components/session-sidebar.test.tsx` - `npm run typecheck -w @open-inspect/control-plane` - `npm run lint -w @open-inspect/control-plane -- --quiet` - `npm run typecheck -w @open-inspect/web` - `npm run lint -w @open-inspect/web -- --quiet` - `npm run build -w @open-inspect/shared` - `npm run build -w @open-inspect/control-plane` - `npm run build -w @open-inspect/web` - `npx prettier --check ...changed TS/TSX files` - `git diff --check` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Sidebar ownership filter (All vs Mine); session list supports filtering by creator (createdBy) and scope=mine resolves the current user. * **Performance** * New DB index to speed creator-filtered session queries. * **Improvements** * Unified session-list cache keys and more reliable cache invalidation for rename, archive/unarchive, and pagination. * **Bug Fixes** * Invalid createdBy or scope now return proper 400 errors. * **Tests** * Added coverage for createdBy/scope handling, cache-key behavior, and related flows. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/688?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> * fix(modal): support non-main Modal environments (ColeMurray#687) ## Summary - Takes over ColeMurray#629 on a new branch because the original PR head is company-owned. - Adds `modal_environment` through the Modal Terraform module so secrets and deploy commands run in the selected Modal environment. - Adds `modal_environment_web_suffix` / `MODAL_ENVIRONMENT_WEB_SUFFIX` for Modal endpoint URL hosts, keeping it separate from the CLI environment name. - Keeps `MODAL_WORKSPACE` as the raw workspace, passes `MODAL_ENVIRONMENT` for dashboard links, and uses the web suffix for Modal API endpoint slugs. - Documents the new Modal environment settings and wires them into Terraform GitHub Actions. ## Addresses Review Feedback - Owner thread on `terraform/environments/production/modal.tf`: Modal CLI now receives `MODAL_ENVIRONMENT` for both secret creation and deploys. - Owner thread on `packages/control-plane/src/sandbox/client.ts`: endpoint hosts now use explicit Modal web suffix instead of deriving from environment name. - CodeRabbit validation note on `modal_environment`: rejects empty values, colons, slashes, and backslashes for Modal deployments, including at the module boundary. - CodeRabbit deploy script note: validates all required env vars before first use under `set -u`. ## Validation - `npm run build -w @open-inspect/shared` - `npm test -w @open-inspect/control-plane -- --run src/sandbox/client.test.ts` - `npm run typecheck -w @open-inspect/control-plane` - `npm run lint -w @open-inspect/control-plane` - `terraform fmt -recursive -check terraform` - `terraform -chdir=terraform/environments/production init -backend=false` - `terraform -chdir=terraform/environments/production validate` - `bash -n terraform/modules/modal-app/scripts/deploy.sh` - `bash -n terraform/modules/modal-app/scripts/create-secrets.sh` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Add selectable Modal deployment environments and pass environment to Modal clients and deploy tooling * Configurable environment-to-endpoint web-suffix for Modal URLs and health checks * **Documentation** * Updated setup, CI, and Terraform docs to document workspace, environment, and web-suffix and new secrets/vars * **Tests** * Added tests for workspace-slug generation and environment-specific client health endpoints <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/687?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Kyle Adams <kadams54@users.noreply.github.com> * fix(slack-bot): cap App Home repo-override list under Slack's block limit (ColeMurray#689) ## Problem The App Home tab renders **one block per repo-specific branch override** with no upper bound (`slack-bot/src/index.ts`). Slack's `views.publish` rejects any view over **100 blocks**, so once a user configures ~85+ repo overrides the entire Home tab fails to publish — including the UI needed to manage those overrides, leaving the user stuck. ## Fix - Cap rendered overrides at **50** (`MAX_RENDERED_REPO_OVERRIDES`), well under the 100-block ceiling (~15 base blocks). - Surface the remainder as a `…and N more` summary instead of dropping it silently. - Hidden overrides stay **fully manageable** via the existing "Search repository" selector → modal, which clears any repo's override on empty submit — so the cap strands nothing. ## Test Adds a regression test that drives the real `app_home_opened` → `views.publish` path with 60 overrides and asserts the published view stays ≤ 100 blocks, renders exactly 50 rows, and includes the "10 more overrides" note. Full slack-bot suite (87 tests) + typecheck pass. Found while syncing this change into a downstream fork. Happy to adjust the cap value or copy. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved handling of large numbers of repository overrides in the Slack App Home interface. The interface now displays up to 50 overrides with a summary note indicating additional hidden overrides, ensuring functionality within Slack's technical constraints. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/689?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> * Refactor Slack App Home module (ColeMurray#690) ## Summary - move Slack App Home publishing, interaction handling, view construction, modals, metadata, model lookup, and local Slack view types into focused `app-home/` modules - extract user preference persistence/resolution into `user-preferences.ts` - keep the `/interactions` route delegating App Home-specific payloads while leaving generic Slack interaction handling in `index.ts` - move App Home view unit coverage into `app-home.test.ts` ## Validation - `npm run typecheck -w @open-inspect/slack-bot` - `npm run lint -w @open-inspect/slack-bot` - `npx prettier --check packages/slack-bot/src/app-home packages/slack-bot/src/app-home.test.ts packages/slack-bot/src/user-preferences.ts` - `git diff --check` - `npm test -w @open-inspect/slack-bot` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * App Home settings: model selection, reasoning-effort choice, global and per-repo branch overrides with modals. * Repo search suggestions and truncated option labels to fit Slack limits; long override lists show a “more” summary. * **Refactor** * Centralized App Home interaction routing and unified preference resolution. * App Home publishing made asynchronous and robust. * **Tests** * Expanded coverage for App Home view, suggestions, truncation, and preference updates. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cole Murray <colemurray.cs@gmail.com> Co-authored-by: Kyle Adams <kadams54@users.noreply.github.com> Co-authored-by: Merlin <40275364+merlin-mk@users.noreply.github.com>
](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/685?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/686?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/688?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/687?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/689?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
app-home/modulesuser-preferences.ts/interactionsroute delegating App Home-specific payloads while leaving generic Slack interaction handling inindex.tsapp-home.test.tsValidation
npm run typecheck -w @open-inspect/slack-botnpm run lint -w @open-inspect/slack-botnpx prettier --check packages/slack-bot/src/app-home packages/slack-bot/src/app-home.test.ts packages/slack-bot/src/user-preferences.tsgit diff --checknpm test -w @open-inspect/slack-botSummary by CodeRabbit
New Features
Refactor
Tests