fix(modal): Support non-main envs in Modal#629
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.
📝 WalkthroughWalkthroughThis PR extends Modal workspace configuration to support environment-specific naming. A new ChangesModal Environment Variable Integration
Sequence DiagramNot applicable; this PR contains configuration and infrastructure variable updates without multi-component runtime interaction. Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
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 `@terraform/environments/production/variables.tf`:
- Around line 75-84: Update the validation block for the Terraform variable
"modal_environment" to also reject colon and slash characters: modify the
existing validation condition in the variable "modal_environment" (currently
referencing var.sandbox_provider and length(var.modal_environment)) to
incorporate a regex check that ensures the value does not contain ":" or "/"
(e.g., using can(regex("^[^:/]+$", var.modal_environment)) in the condition when
sandbox_provider == "modal"), and update the error_message to something like
"modal_environment must not contain colons or slashes." so invalid endpoint
slugs are prevented.
🪄 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: 29cf18c9-edff-40e9-938f-637f10338315
📒 Files selected for processing (6)
docs/GETTING_STARTED.mdterraform/environments/production/locals.tfterraform/environments/production/modal.tfterraform/environments/production/terraform.tfvars.exampleterraform/environments/production/variables.tfterraform/environments/production/workers-control-plane.tf
|
|
||
| app_name = "open-inspect" | ||
| workspace = var.modal_workspace | ||
| workspace = local.modal_workspace_slug |
There was a problem hiding this comment.
[Automated Review]
The slug is plumbed into URL construction here, but modal deploy / modal secret create in terraform/modules/modal-app/scripts/{deploy,create-secrets}.sh are invoked without --env (and no MODAL_ENVIRONMENT is exported), so they deploy to the workspace's default environment regardless of var.modal_environment.
Concretely, with modal_environment = "production":
- This module's outputs and the worker's
MODAL_WORKSPACE→myworkspace-production--…modal.run - But
modal deploylands the app at the workspace default (typicallymain) →myworkspace--…modal.run - Result: every endpoint 404s.
Suggested fix:
- Add a
modal_environmentinput to themodal-appmodule. - Pass it as
MODAL_ENVIRONMENTin theenvironmentblocks ofnull_resource.modal_secretsandnull_resource.modal_deployinmodules/modal-app/main.tf— Modal CLI reads that var natively.
(Or alternatively, append--env "${MODAL_ENVIRONMENT}"in both scripts.)
Without this, the feature is half-wired for non-main envs — URLs point one place, deployment lands somewhere else.
ColeMurray
left a comment
There was a problem hiding this comment.
see comment regarding modal script
## Summary - add a provider-neutral `sandbox_dashboard_url` server message and session state field - broadcast the dashboard URL after a sandbox provider object ID is persisted on spawn, restore, or persistent resume - render the desktop sandbox status as an external provider-dashboard link when a URL is available ## Why Modal exposes useful per-sandbox diagnostics in its dashboard, but the web UI currently has no way to link users from a session to the corresponding provider sandbox. This makes debugging stuck, slow, or failed sandboxes harder than it needs to be. The implementation keeps the shared protocol provider-neutral and confines Modal URL construction to the control-plane Modal path. Existing connected clients receive the URL as soon as the provider object ID is known, while newly subscribed clients receive it through the hydrated session state. ## Prior upstream work checked - refreshed `upstream/main` before opening this PR; this branch is one commit ahead and zero commits behind - checked `upstream/main` for `sandboxDashboardUrl`, `sandbox_dashboard_url`, and `modal_sandbox_url`; no existing implementation was present - searched existing upstream PRs for sandbox/dashboard/Modal dashboard terms; no matching fix for this URL plumbing was found - noted open PR #629 for non-main Modal environments; this PR defaults the Modal dashboard environment to `main` and does not add new Terraform/env wiring ## Details - `buildModalSandboxDashboardUrl` returns `null` unless the required Modal workspace and provider object ID are present. - `SandboxLifecycleManager` centralizes provider object ID persistence and broadcasts the dashboard URL only when configured. - `SessionDO` wires the builder only for the Modal backend. - The web socket hook clears stale dashboard URLs when a replacement sandbox starts or the current sandbox reaches a terminal state. ## Tests - `npm run build -w @open-inspect/shared` - `npm test -w @open-inspect/control-plane` - `npm test -w @open-inspect/web` - `npm run typecheck -w @open-inspect/control-plane` - `npm run typecheck -w @open-inspect/web` - `npm run lint` - `npm run typecheck` - `npm test` - `git diff --check` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Session header shows a clickable sandbox provider dashboard link when available; session state and server messages now include an optional sandbox dashboard URL. * **Bug Fixes** * Dashboard link visibility and updates are more consistent across spawn, restore, and resume flows; dashboard URL cleared or retained appropriately during lifecycle transitions. * **Tests** * Added tests for URL construction, message delivery, session-state hydration, and sandbox access state transitions. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/654?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 -->
…27) * chore(deps-dev): bump qs from 6.15.1 to 6.15.2 in the npm_and_yarn group across 1 directory (ColeMurray#678) Bumps the npm_and_yarn group with 1 update in the / directory: [qs](https://github.com/ljharb/qs). Updates `qs` from 6.15.1 to 6.15.2 <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/ljharb/qs/blob/main/CHANGELOG.md">qs's changelog</a>.</em></p> <blockquote> <h2><strong>6.15.2</strong></h2> <ul> <li>[Fix] <code>stringify</code>: skip null/undefined entries in <code>arrayFormat: 'comma'</code> + <code>encodeValuesOnly</code> instead of crashing in <code>encoder</code></li> <li>[Fix] <code>stringify</code>: use configured <code>delimiter</code> after <code>charsetSentinel</code> (<a href="https://redirect.github.com/ljharb/qs/issues/555">#555</a>)</li> <li>[Fix] <code>stringify</code>: apply <code>formatter</code> to encoded key under <code>strictNullHandling</code> (<a href="https://redirect.github.com/ljharb/qs/issues/554">#554</a>)</li> <li>[Fix] <code>stringify</code>: skip null/undefined filter-array entries instead of crashing in <code>encoder</code> (<a href="https://redirect.github.com/ljharb/qs/issues/551">#551</a>)</li> <li>[Fix] <code>parse</code>: handle nested bracket groups and add regression tests (<a href="https://redirect.github.com/ljharb/qs/issues/530">#530</a>)</li> <li>[readme] fix grammar (<a href="https://redirect.github.com/ljharb/qs/issues/550">#550</a>)</li> <li>[Dev Deps] update <code>@ljharb/eslint-config</code></li> <li>[Tests] add regression tests for keys containing percent-encoded bracket text</li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/ljharb/qs/commit/9aca4076fe788338c67cf7e115f0be6bc58d85a8"><code>9aca407</code></a> v6.15.2</li> <li><a href="https://github.com/ljharb/qs/commit/5e33d33447ed0bf1ddab9abc41d27dea4687d992"><code>5e33d33</code></a> [Dev Deps] update <code>@ljharb/eslint-config</code></li> <li><a href="https://github.com/ljharb/qs/commit/21f80b33e5c8b3f7eba1034fff0da4a4a37a1d41"><code>21f80b3</code></a> [Fix] <code>stringify</code>: skip null/undefined entries in <code>arrayFormat: 'comma'</code> + `e...</li> <li><a href="https://github.com/ljharb/qs/commit/a0a81ea2071acce3eff41a040f719ac8f5c4f64c"><code>a0a81ea</code></a> [Fix] <code>stringify</code>: use configured <code>delimiter</code> after <code>charsetSentinel</code></li> <li><a href="https://github.com/ljharb/qs/commit/e3062f78f5233b338ceeb8e8dfa5a07dea4b32a8"><code>e3062f7</code></a> [Fix] <code>stringify</code>: apply <code>formatter</code> to encoded key under <code>strictNullHandling</code></li> <li><a href="https://github.com/ljharb/qs/commit/0c180a40adb8c6703fffc85b2ff06ca209f5c1e0"><code>0c180a4</code></a> [Fix] <code>stringify</code>: skip null/undefined filter-array entries instead of crashi...</li> <li><a href="https://github.com/ljharb/qs/commit/3a8b94aec19bd664720f6f6b1e66c4a0dfe4b656"><code>3a8b94a</code></a> [Tests] add regression tests for keys containing percent-encoded bracket text</li> <li><a href="https://github.com/ljharb/qs/commit/96755abd357c0e534dd3442a84a04d08864bfe0d"><code>96755ab</code></a> [readme] fix grammar</li> <li><a href="https://github.com/ljharb/qs/commit/a419ce5bbfcdb98a299f1a0bb47ea055baef20e6"><code>a419ce5</code></a> [Fix] <code>parse</code>: handle nested bracket groups and add regression tests</li> <li>See full diff in <a href="https://github.com/ljharb/qs/compare/v6.15.1...v6.15.2">compare view</a></li> </ul> </details> <br /> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore <dependency name> major version` will close this group update PR and stop Dependabot creating any more for the specific dependency's major version (unless you unignore this specific dependency's major version or upgrade to it yourself) - `@dependabot ignore <dependency name> minor version` will close this group update PR and stop Dependabot creating any more for the specific dependency's minor version (unless you unignore this specific dependency's minor version or upgrade to it yourself) - `@dependabot ignore <dependency name>` will close this group update PR and stop Dependabot creating any more for the specific dependency (unless you unignore this specific dependency or upgrade to it yourself) - `@dependabot unignore <dependency name>` will remove all of the ignore conditions of the specified dependency - `@dependabot unignore <dependency name> <ignore condition>` will remove the ignore condition of the specified dependency and ignore conditions You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/ColeMurray/background-agents/network/alerts). </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * feat(sandbox): add provider dashboard links (ColeMurray#654) ## Summary - add a provider-neutral `sandbox_dashboard_url` server message and session state field - broadcast the dashboard URL after a sandbox provider object ID is persisted on spawn, restore, or persistent resume - render the desktop sandbox status as an external provider-dashboard link when a URL is available ## Why Modal exposes useful per-sandbox diagnostics in its dashboard, but the web UI currently has no way to link users from a session to the corresponding provider sandbox. This makes debugging stuck, slow, or failed sandboxes harder than it needs to be. The implementation keeps the shared protocol provider-neutral and confines Modal URL construction to the control-plane Modal path. Existing connected clients receive the URL as soon as the provider object ID is known, while newly subscribed clients receive it through the hydrated session state. ## Prior upstream work checked - refreshed `upstream/main` before opening this PR; this branch is one commit ahead and zero commits behind - checked `upstream/main` for `sandboxDashboardUrl`, `sandbox_dashboard_url`, and `modal_sandbox_url`; no existing implementation was present - searched existing upstream PRs for sandbox/dashboard/Modal dashboard terms; no matching fix for this URL plumbing was found - noted open PR ColeMurray#629 for non-main Modal environments; this PR defaults the Modal dashboard environment to `main` and does not add new Terraform/env wiring ## Details - `buildModalSandboxDashboardUrl` returns `null` unless the required Modal workspace and provider object ID are present. - `SandboxLifecycleManager` centralizes provider object ID persistence and broadcasts the dashboard URL only when configured. - `SessionDO` wires the builder only for the Modal backend. - The web socket hook clears stale dashboard URLs when a replacement sandbox starts or the current sandbox reaches a terminal state. ## Tests - `npm run build -w @open-inspect/shared` - `npm test -w @open-inspect/control-plane` - `npm test -w @open-inspect/web` - `npm run typecheck -w @open-inspect/control-plane` - `npm run typecheck -w @open-inspect/web` - `npm run lint` - `npm run typecheck` - `npm test` - `git diff --check` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Session header shows a clickable sandbox provider dashboard link when available; session state and server messages now include an optional sandbox dashboard URL. * **Bug Fixes** * Dashboard link visibility and updates are more consistent across spawn, restore, and resume flows; dashboard URL cleared or retained appropriately during lifecycle transitions. * **Tests** * Added tests for URL construction, message delivery, session-state hydration, and sandbox access state transitions. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/654?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 --> * feat: Expose opencode's auto generated session titles (ColeMurray#569) Expose opencode's autogenerated session titles to open inspect. It will get the title after the first turn and rename the session if a session has not been manually named. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Runtime-sourced session titles are fetched and emitted as real-time session_title events; clients update session title and sidebar session list is revalidated. * **Bug Fixes** * Generated titles are sanitized and applied only when appropriate; title updates respect monotonic timestamps and ignore stale writes. Background index synchronization and client broadcast added. * **Tests** * Added tests covering title emission, conditional persistence, index sync, and client-side update/revalidation. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/569?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: Kartikye <kartikye@dots.dev> Co-authored-by: Cole Murray <colemurray.cs@gmail.com> * fix: don't let status writes suppress session-title index sync updateTitleIfNewer gated the title write on the shared sessions.updated_at column, which is also advanced by unrelated status/touch writes. A generated session_title (synced via ctx.waitUntil) and a later execution_complete status write race to D1; if the status write lands first it bumps updated_at, and the title write's `updated_at <= ?` predicate then drops the title permanently, leaving the sidebar/index title stale. Add a dedicated title_updated_at column (migration 0022) and gate title writes on it instead, while still advancing updated_at monotonically via max() so the session-list sort order stays fresh. A genuinely stale title (older than the last title write) is still ignored. Flagged by Codex review on #27. * test: de-flake dashboard-url hydration integration test The websocket-client "subscribe hydrates dashboard URL" test (added in the ColeMurray#654 sync) seeded sandbox.status via queryDO immediately after initNamedSession. Init fires a background warmSandbox() that, with no Modal in CI, fails fast and overwrites the status with "failed" — racing the seed and the subscribe read (it passed locally, failed in CI with expected 'failed' to be 'stopped'). Wait for the warm spawn to settle (status "failed") before seeding, the same guard websocket-sandbox.test.ts already uses. Subscribe never re-warms, so the seeded status is then stable. --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Kartikye Mittal <kartikye.mittal@gmail.com> Co-authored-by: Kartikye <kartikye@dots.dev> Co-authored-by: Cole Murray <colemurray.cs@gmail.com>
## Summary - Takes over #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>
* 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>
Summary
Modal has a hierarchy of workspaces -> environments. The default environment is
main. If you stick with that, the environment doesn't have any impact on Modal's URLs; however, if you use a non-default environment, e.g.,prod, that environment name is added into the URL. These changes support generating Modal's URL slug properly to support bothmainand non-default environments.Backwards compatibility
modal_environmentTerraform variable defaults tomainso projects that don't set it will not be impacted.Docs updated
docs/GETTING_STARTED.md— updatedterraform.tfvarsblockterraform/environments/production/terraform.tfvars.example— updated example to include the new variableSummary by CodeRabbit
Documentation
New Features