fix(modal): support non-main Modal environments#687
Merged
Conversation
modal_workspace held the full URL slug rather than just the workspace name. Added modal_environment and a modal_workspace_slug local so each variable holds its correct semantic value.
📝 WalkthroughWalkthroughAdds first-class Modal environment support: client library and tests accept an environment/web-suffix, control plane passes environment bindings to Modal clients and dashboard URLs, Terraform defines/validates modal_environment and modal_environment_web_suffix and wires them through module triggers and scripts, CI/workflow and docs updated. ChangesModal Environment Configuration and Deployment
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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}
![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
🧹 Nitpick comments (2)
terraform/modules/modal-app/variables.tf (1)
23-27: ⚡ Quick winAdd module-level validation for
modal_environment.This module input is externally callable, so it should reject empty values and invalid path delimiters at the module boundary too (not only in the production wrapper).
Proposed diff
variable "modal_environment" { description = "Modal environment name used by the Modal CLI" type = string default = "main" + + validation { + condition = length(var.modal_environment) > 0 && can(regex("^[^:/]+$", var.modal_environment)) + error_message = "modal_environment must be non-empty and must not contain colons or slashes." + } }🤖 Prompt for 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. In `@terraform/modules/modal-app/variables.tf` around lines 23 - 27, The variable "modal_environment" lacks module-level validation allowing empty strings or path delimiters; add a Terraform validation block on variable "modal_environment" that enforces a non-empty value (trimmed length > 0) and rejects values containing path delimiters (e.g., "/" or "\") by using a condition like can(regex(...)) or (!contains(regexall("[/\\\\]", var.modal_environment), [])) inverted appropriately, and provide a clear error_message indicating the value must be non-empty and must not contain path delimiters; place this validation inside the existing variable "modal_environment" block.packages/control-plane/src/sandbox/client.ts (1)
21-29: 💤 Low valueConsider removing redundant null-coalescing on line 25.
The parameter
environmentalready defaults toDEFAULT_MODAL_ENVIRONMENT(line 23), so the|| DEFAULT_MODAL_ENVIRONMENTon line 25 only handles the case where an empty string is explicitly passed. Since callers likebuildModalSandboxDashboardUrlalready filter empty strings before calling this function, the extra check is redundant.♻️ Proposed simplification
export function buildModalWorkspaceSlug( workspace: string, environment = DEFAULT_MODAL_ENVIRONMENT ): string { - const modalEnvironment = environment || DEFAULT_MODAL_ENVIRONMENT; - return modalEnvironment === DEFAULT_MODAL_ENVIRONMENT + return environment === DEFAULT_MODAL_ENVIRONMENT ? workspace - : `${workspace}-${modalEnvironment}`; + : `${workspace}-${environment}`; }🤖 Prompt for 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. In `@packages/control-plane/src/sandbox/client.ts` around lines 21 - 29, The modalEnvironment null-coalescing is redundant: inside buildModalWorkspaceSlug the parameter environment already defaults to DEFAULT_MODAL_ENVIRONMENT, so remove the unnecessary "environment || DEFAULT_MODAL_ENVIRONMENT" usage (or the separate modalEnvironment variable) and use environment directly when comparing to DEFAULT_MODAL_ENVIRONMENT; ensure callers such as buildModalSandboxDashboardUrl that filter empty strings remain unchanged.
🤖 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 `@terraform/modules/modal-app/scripts/deploy.sh`:
- Around line 29-33: Echoing environment variables (APP_NAME, DEPLOY_PATH,
DEPLOY_MODULE, MODAL_ENVIRONMENT) before validating them can trigger
unbound-variable errors under set -u; update deploy.sh to validate all required
env vars at the top (check APP_NAME, DEPLOY_PATH, DEPLOY_MODULE and optionally
MODAL_ENVIRONMENT) and exit with a clear error message if any are missing, then
perform the echo lines, ensuring checks occur before any reference to those
variables.
---
Nitpick comments:
In `@packages/control-plane/src/sandbox/client.ts`:
- Around line 21-29: The modalEnvironment null-coalescing is redundant: inside
buildModalWorkspaceSlug the parameter environment already defaults to
DEFAULT_MODAL_ENVIRONMENT, so remove the unnecessary "environment ||
DEFAULT_MODAL_ENVIRONMENT" usage (or the separate modalEnvironment variable) and
use environment directly when comparing to DEFAULT_MODAL_ENVIRONMENT; ensure
callers such as buildModalSandboxDashboardUrl that filter empty strings remain
unchanged.
In `@terraform/modules/modal-app/variables.tf`:
- Around line 23-27: The variable "modal_environment" lacks module-level
validation allowing empty strings or path delimiters; add a Terraform validation
block on variable "modal_environment" that enforces a non-empty value (trimmed
length > 0) and rejects values containing path delimiters (e.g., "/" or "\") by
using a condition like can(regex(...)) or (!contains(regexall("[/\\\\]",
var.modal_environment), [])) inverted appropriately, and provide a clear
error_message indicating the value must be non-empty and must not contain path
delimiters; place this validation inside the existing variable
"modal_environment" block.
🪄 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: 208b07f5-a791-4283-a469-7ad879826192
📒 Files selected for processing (18)
.github/workflows/terraform.ymldocs/GETTING_STARTED.mdpackages/control-plane/src/routes/repo-images.tspackages/control-plane/src/sandbox/client.test.tspackages/control-plane/src/sandbox/client.tspackages/control-plane/src/sandbox/providers/modal-provider.tspackages/control-plane/src/session/durable-object.tspackages/control-plane/src/types.tsterraform/README.mdterraform/environments/production/locals.tfterraform/environments/production/modal.tfterraform/environments/production/terraform.tfvars.exampleterraform/environments/production/variables.tfterraform/environments/production/workers-control-plane.tfterraform/modules/modal-app/main.tfterraform/modules/modal-app/scripts/create-secrets.shterraform/modules/modal-app/scripts/deploy.shterraform/modules/modal-app/variables.tf
Contributor
There was a problem hiding this comment.
Summary
PR #687, fix(modal): support non-main Modal environments, by @ColeMurray. Reviewed 18 changed files (+158/-38); the plumbing for MODAL_ENVIRONMENT through Terraform, GitHub Actions, scripts, and the control plane is mostly cohesive, but there is a blocking correctness issue in how Modal endpoint URLs are derived.
Critical Issues
- [Correctness]
packages/control-plane/src/sandbox/client.ts:24- Modal endpoint URLs use the environment web suffix, not necessarily the environment name. The control plane and Terraform currently deriveworkspace-environment--...fromMODAL_ENVIRONMENT, which can target the wrong host for environments whose web suffix differs from the environment name, formainenvironments with a non-empty suffix, or for another environment with the empty suffix. I left an inline comment with suggested fixes.
Suggestions
None beyond the blocking URL-source issue.
Nitpicks
None.
Positive Feedback
- The change consistently threads
MODAL_ENVIRONMENTinto Modal deploys, secret creation, control-plane bindings, and repo-image client creation. - The scripts now fail fast when the Modal environment is missing, which avoids silently deploying/seeding secrets into the wrong environment.
- The added tests cover default
mainbehavior, non-main endpoint generation, dashboard URLs, and URL component encoding.
Questions
None.
Verdict
Request Changes: please separate the CLI environment name from the Modal endpoint web suffix/source slug, or otherwise use a Modal-provided deployed URL instead of deriving endpoint hosts from MODAL_ENVIRONMENT.
Terraform Validation Results
Pushed by: @ColeMurray, Action: |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@terraform/environments/production/variables.tf`:
- Around line 92-93: The validation regex for modal_environment_web_suffix
currently only forbids colons, slashes, and backslashes but allows internal
whitespace; update the regex in the condition expression to also reject any
whitespace by changing regex("^[^:/\\\\]+$", var.modal_environment_web_suffix)
to regex("^[^[:space:]:/\\\\]+$", var.modal_environment_web_suffix) (keep the
surrounding logic and error_message the same) so values like "prod env" are
rejected.
In `@terraform/modules/modal-app/variables.tf`:
- Around line 40-41: Update the validation for modal_environment_web_suffix to
reject any whitespace characters; replace the current complex
trimspace/length/regex check with a single regex that forbids whitespace,
colons, slashes and backslashes. For example, change the condition to allow
empty string or can(regex("^[^\\s:/\\\\]+$", var.modal_environment_web_suffix))
so modal_environment_web_suffix either is "" or matches the
no-whitespace/no-colon/no-slash/no-backslash pattern.
🪄 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: 2236873d-df4a-4254-89fe-4c5a4cdecce0
📒 Files selected for processing (17)
.github/workflows/terraform.ymldocs/GETTING_STARTED.mdpackages/control-plane/src/routes/repo-images.tspackages/control-plane/src/sandbox/client.test.tspackages/control-plane/src/sandbox/client.tspackages/control-plane/src/sandbox/providers/modal-provider.tspackages/control-plane/src/session/durable-object.tspackages/control-plane/src/types.tsterraform/README.mdterraform/environments/production/modal.tfterraform/environments/production/terraform.tfvars.exampleterraform/environments/production/variables.tfterraform/environments/production/workers-control-plane.tfterraform/modules/modal-app/main.tfterraform/modules/modal-app/outputs.tfterraform/modules/modal-app/scripts/deploy.shterraform/modules/modal-app/variables.tf
✅ Files skipped from review due to trivial changes (3)
- terraform/modules/modal-app/outputs.tf
- packages/control-plane/src/sandbox/providers/modal-provider.ts
- docs/GETTING_STARTED.md
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/control-plane/src/types.ts
- .github/workflows/terraform.yml
- terraform/modules/modal-app/scripts/deploy.sh
- terraform/modules/modal-app/main.tf
- terraform/environments/production/workers-control-plane.tf
- packages/control-plane/src/sandbox/client.ts
- packages/control-plane/src/sandbox/client.test.ts
Terraform Validation Results
Pushed by: @ColeMurray, Action: |
Owner
Author
|
Addressed the two follow-ups in 5356dc5.
|
Terraform Validation Results
Pushed by: @ColeMurray, Action: |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/GETTING_STARTED.md (2)
363-363: ⚡ Quick winConsider showing empty string as the default example.
The comment correctly notes that empty is the value for default endpoints (
https://workspace--...), but the example shows a non-empty placeholder. Since most users start with the default Modal environment, showingmodal_environment_web_suffix = ""as the example would be clearer.📝 Suggested improvement
-modal_environment_web_suffix = "your-modal-web-suffix" # Lowercase letters, digits, dashes; empty for https://workspace--... endpoints +modal_environment_web_suffix = "" # Lowercase letters, digits, dashes; empty for https://workspace--... endpoints (use non-empty for custom environments)🤖 Prompt for 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. In `@docs/GETTING_STARTED.md` at line 363, Update the example for modal_environment_web_suffix to show the empty-string default (modal_environment_web_suffix = "") so users see the typical default for https://workspace--... endpoints; locate and update the placeholder in the docs where modal_environment_web_suffix is defined to use "" instead of "your-modal-web-suffix".
637-641: 💤 Low valueClarify the hyphen separator in the workspace slug example.
Line 640's comment shows
"YOUR-WORKSPACE-YOUR-MODAL-WEB-SUFFIX", but it's ambiguous whether there's a single hyphen separator or if the placeholder names themselves include hyphens. Consider making the separator explicit.📝 Suggested improvement
-MODAL_WORKSPACE_SLUG="YOUR-WORKSPACE" # or "YOUR-WORKSPACE-YOUR-MODAL-WEB-SUFFIX" +MODAL_WORKSPACE_SLUG="YOUR-WORKSPACE" # or "YOUR-WORKSPACE-YOUR-SUFFIX" (workspace and suffix joined by single hyphen)🤖 Prompt for 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. In `@docs/GETTING_STARTED.md` around lines 637 - 641, Clarify the hyphen separator in the MODAL_WORKSPACE_SLUG example by replacing the ambiguous "YOUR-WORKSPACE-YOUR-MODAL-WEB-SUFFIX" with an explicit form that shows the separator between the two placeholders (e.g., "YOUR-WORKSPACE - YOUR-MODAL-WEB-SUFFIX" or "YOUR-WORKSPACE<hyphen>YOUR-MODAL-WEB-SUFFIX") so readers understand the hyphen is the separator between two distinct placeholders; update the comment and the example value for MODAL_WORKSPACE_SLUG to use the chosen explicit notation.
🤖 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.
Nitpick comments:
In `@docs/GETTING_STARTED.md`:
- Line 363: Update the example for modal_environment_web_suffix to show the
empty-string default (modal_environment_web_suffix = "") so users see the
typical default for https://workspace--... endpoints; locate and update the
placeholder in the docs where modal_environment_web_suffix is defined to use ""
instead of "your-modal-web-suffix".
- Around line 637-641: Clarify the hyphen separator in the MODAL_WORKSPACE_SLUG
example by replacing the ambiguous "YOUR-WORKSPACE-YOUR-MODAL-WEB-SUFFIX" with
an explicit form that shows the separator between the two placeholders (e.g.,
"YOUR-WORKSPACE - YOUR-MODAL-WEB-SUFFIX" or
"YOUR-WORKSPACE<hyphen>YOUR-MODAL-WEB-SUFFIX") so readers understand the hyphen
is the separator between two distinct placeholders; update the comment and the
example value for MODAL_WORKSPACE_SLUG to use the chosen explicit notation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e5241825-de1a-4d12-9d5f-ea83301821e9
📒 Files selected for processing (4)
docs/GETTING_STARTED.mdterraform/README.mdterraform/environments/production/variables.tfterraform/modules/modal-app/variables.tf
✅ Files skipped from review due to trivial changes (1)
- terraform/README.md
🚧 Files skipped from review as they are similar to previous changes (2)
- terraform/modules/modal-app/variables.tf
- terraform/environments/production/variables.tf
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
2 participants
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
modal_environmentthrough the Modal Terraform module so secrets and deploy commands run in the selected Modal environment.modal_environment_web_suffix/MODAL_ENVIRONMENT_WEB_SUFFIXfor Modal endpoint URL hosts, keeping it separate from the CLI environment name.MODAL_WORKSPACEas the raw workspace, passesMODAL_ENVIRONMENTfor dashboard links, and uses the web suffix for Modal API endpoint slugs.Addresses Review Feedback
terraform/environments/production/modal.tf: Modal CLI now receivesMODAL_ENVIRONMENTfor both secret creation and deploys.packages/control-plane/src/sandbox/client.ts: endpoint hosts now use explicit Modal web suffix instead of deriving from environment name.modal_environment: rejects empty values, colons, slashes, and backslashes for Modal deployments, including at the module boundary.set -u.Validation
npm run build -w @open-inspect/sharednpm test -w @open-inspect/control-plane -- --run src/sandbox/client.test.tsnpm run typecheck -w @open-inspect/control-planenpm run lint -w @open-inspect/control-planeterraform fmt -recursive -check terraformterraform -chdir=terraform/environments/production init -backend=falseterraform -chdir=terraform/environments/production validatebash -n terraform/modules/modal-app/scripts/deploy.shbash -n terraform/modules/modal-app/scripts/create-secrets.shSummary by CodeRabbit
New Features
Documentation
Tests