feat(shared): slack integration settings types + validation#607
Conversation
Defines `SlackGlobalSettings` (mentions policy + master switch) and `SlackRepoSettings` (master switch only) and wires `"slack"` into the `IntegrationId` union, `IntegrationSettingsMap`, and `INTEGRATION_DEFINITIONS`. Adds level-aware validation in `IntegrationSettingsStore`: global writes accept `mentionsPolicy`; per-repo writes reject it (and any other unknown field). Both levels reject non-boolean `agentNotificationsEnabled` and out-of-range `mentionsPolicy` values. No D1 migration: `integration_settings` and `integration_repo_settings` already store opaque JSON keyed by `integration_id`. Field names use camelCase (`agentNotificationsEnabled`, `mentionsPolicy`) to match existing settings (`autoReviewOnOpen`, `tunnelPorts`, etc.); the spec's snake_case is treated as pseudocode.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds Slack integration types and a Slack entry in integration definitions; threads a SettingsLevel into validateAndNormalizeSettings; implements validateSlackSettings enforcing level-specific allowed keys and value constraints; updates setGlobal/setRepoSettings to pass level; adjusts getResolvedConfig typing; and adds comprehensive Slack tests. ChangesSlack Integration Settings Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 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: |
There was a problem hiding this comment.
Summary
PR #607, feat(shared): slack integration settings types + validation, by @ColeMurray adds Slack integration settings types in shared and wires Slack validation through IntegrationSettingsStore. I reviewed the 3 changed files (+190/-5) and found one blocking type-contract mismatch; inline comment posted.
Critical Issues
- [Functionality & Correctness]
packages/shared/src/types/integrations.ts:68-IntegrationSettingsMap["slack"]["repo"]is still typed asSlackGlobalSettings, so downstream callers can type-check repo payloads containingmentionsPolicyeven though the new store validation rejects that field at runtime. This makes the exported shared types disagree with the actual API contract for per-repo Slack settings.
Suggestions
- None.
Nitpicks
- None.
Positive Feedback
- The new store-level validation is straightforward and consistently enforces the allowed Slack keys and value shapes.
- The Slack test coverage is solid for both acceptance and rejection paths, including resolved-config behavior.
- Reusing the existing integration settings plumbing keeps the change small and easy to follow.
Questions
- None.
Verdict
Request Changes: the shared repo-level Slack type should match the runtime contract before this lands.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/shared/src/types/integrations.ts (1)
62-69: 🏗️ Heavy liftSlack repo typing is broader than intended (global-only field allowed at compile time).
IntegrationEntry<SlackGlobalSettings>makesrepoacceptmentionsPolicy, so invalid repo payloads type-check and only fail at runtime validation. Consider modeling Slack with distinct global/repo shapes inIntegrationSettingsMapto enforce the per-level contract at compile time too.♻️ Suggested direction
export interface IntegrationSettingsMap { github: IntegrationEntry<GitHubBotSettings>; linear: IntegrationEntry<LinearBotSettings>; "code-server": IntegrationEntry<CodeServerSettings>; sandbox: IntegrationEntry<SandboxSettings>; - slack: IntegrationEntry<SlackGlobalSettings>; + slack: { + global: { + enabledRepos?: string[]; + defaults?: SlackGlobalSettings; + }; + repo: SlackRepoSettings; + }; }🤖 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/shared/src/types/integrations.ts` around lines 62 - 69, IntegrationSettingsMap currently uses IntegrationEntry<SlackGlobalSettings> which lets repo-level data include global-only fields like mentionsPolicy; define a separate SlackRepoSettings type (omitting mentionsPolicy and any other global-only fields) and change the Slack entry to use the IntegrationEntry specialization that accepts distinct global and repo shapes (e.g., IntegrationEntry<SlackGlobalSettings, SlackRepoSettings>), updating imports/exports accordingly so repo payloads are statically constrained to the repo shape.
🤖 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 `@packages/shared/src/types/integrations.ts`:
- Around line 62-69: IntegrationSettingsMap currently uses
IntegrationEntry<SlackGlobalSettings> which lets repo-level data include
global-only fields like mentionsPolicy; define a separate SlackRepoSettings type
(omitting mentionsPolicy and any other global-only fields) and change the Slack
entry to use the IntegrationEntry specialization that accepts distinct global
and repo shapes (e.g., IntegrationEntry<SlackGlobalSettings,
SlackRepoSettings>), updating imports/exports accordingly so repo payloads are
statically constrained to the repo shape.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1d7d7f0a-f711-4d2f-bf0d-a874de1ee9da
📒 Files selected for processing (3)
packages/control-plane/src/db/integration-settings.test.tspackages/control-plane/src/db/integration-settings.tspackages/shared/src/types/integrations.ts
…ile time
`IntegrationSettingsMap["slack"]["repo"]` resolved to `SlackGlobalSettings`,
so per-repo payloads carrying `mentionsPolicy` type-checked but failed at
runtime in `validateSlackSettings(_, "repo")`. Make the static contract
match the runtime one.
- Extend `IntegrationEntry<TRepo>` with a second optional generic
`TGlobalDefaults` (defaults to `TRepo`); the four existing integrations
are unchanged.
- Slack now uses `IntegrationEntry<SlackRepoSettings, SlackGlobalSettings>`.
- Widen `getResolvedConfig`'s return type to
`NonNullable<IntegrationSettingsMap[K]["global"]["defaults"]>` so the
resolved settings include global-only fields (e.g. slack
`mentionsPolicy`). For non-slack integrations this is identical to the
previous `["repo"]` type.
`setRepoSettings("slack", repo, { mentionsPolicy: "escape" })` now fails
with TS2353 instead of `IntegrationSettingsValidationError`.
Terraform Validation Results
Pushed by: @ColeMurray, Action: |
## Summary PR 3 of the agent-slack-notification feature ([implementation plan](docs/agent-slack-notification-tool-implementation-plan.md)). Operator-facing surface for the Slack integration shipped in #607. Three controls, mirroring the pattern of `github-integration-settings.tsx`: - **Master switch** (global): `agentNotificationsEnabled`, off by default. - **Mentions policy** (global, radio: allow / escape / strip). Workspace-wide; not overridable per repo (per spec §9.1). - **Per-repo override** (`inherit` / `on` / `off`): forces the master switch for a specific repo. Help copy explains that channel access is delegated to Slack bot membership rather than an Open-Inspect-side allowlist (per the v3 plan's allowlist-removal decision). The existing `[id]` API proxy already passes `slack` through to the control plane, so no new web-side API routes are needed. ## What this does NOT do - No control-plane endpoint or sandbox tool yet — those land in PRs 4 and 5. Until then, the UI writes settings the control plane will read once PR 4 wires the feature flag. - No event-list rendering for `slack-notify` tool calls — that's PR 6. ## Test plan - [x] `npm test -w @open-inspect/web` — 217/217 (9 new component tests for `<SlackIntegrationSettings />`) - [x] `npm run typecheck -w @open-inspect/web` - [x] `npm run lint -w @open-inspect/web` - [ ] Manual browser smoke: navigate to Settings → Integrations → Slack, configure settings, save, reload, confirm settings persisted (will verify after this and #607 are deployed together) ## Rollback Revert. Operators can still hit the API directly with curl if needed; PR 4 still works. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added Slack integration settings UI with workspace defaults and per-repository overrides; configure agent notifications and mentions policy; add/remove repo overrides and reset to defaults with confirmation. * **Improvements** * Loading skeleton, help text, and clearer save/reset feedback; repository picker dedupes mixed-case entries. * **Tests** * Expanded test coverage for loading, save/reset flows, per-repo overrides, and server-sync/resync behaviors. [](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/608) <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…name) (#5) * refactor: extract SessionInitInput type and initializeSession (#587) ## Summary - Introduces a shared `SessionInitInput` interface and `initializeSession()` function in `session/initialize.ts` that both `handleCreateSession` and `handleSpawnChild` now call - Enforces D1-first ordering as an invariant — D1 index is written before the DO is initialized (previously `handleSpawnChild` had DO-first ordering, which could leave orphaned DOs on D1 failure) - Closes the untyped DO init body gap — the `SessionInitInput` type is shared between the router and the DO handler, preventing field drift - Uses domain-meaningful identity naming: `participantUserId` (session protocol identity for the creator) and `platformUserId` (canonical user for analytics attribution) ### What changed in `router.ts` Both `handleCreateSession` (~217 lines) and `handleSpawnChild` (~221 lines) had duplicate D1 write + DO init sequences with untyped `JSON.stringify` bodies. Each now builds a `SessionInitInput` from its own preconditions and calls `initializeSession()`. Handler-specific logic (identity resolution, enrichment, guardrails, prompt enqueue) stays in the handlers. ### Net effect | Metric | Before | After | |---|---|---| | DO init body type shared? | No — convention only | Yes — `SessionInitInput` | | D1/DO ordering consistent? | No — create=D1-first, spawn=DO-first | Yes — D1-first always | | Lines in `router.ts` | ~2,200 | ~2,087 | | Independently testable? | No | Yes — `initializeSession` has 8 unit tests | ## Test plan - [x] 8 new unit tests for `initializeSession` covering: D1-before-DO ordering, D1 failure prevents DO init, DO init failure throws, correct fields to D1 and DO, correlation headers, return value, branch fallback logic - [x] All 1044 control-plane tests pass (including existing router and lifecycle handler tests) - [x] Typecheck passes - [x] Lint passes <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Consolidated session initialization into a single flow, improving reliability and consistency for session creation and child-session spawning. * **Tests** * Added comprehensive tests covering session initialization, success/failure paths, and data mapping to ensure robust behavior. * **Documentation** * Clarified internal endpoint request structure and field mappings to aid maintainability and debugging. <!-- end of auto-generated comment: release notes by coderabbit.ai --> * feat: configurable app name and icon for whitelabel deployments (#594) ## Summary - Threads a single Terraform variable `app_name` (default `Open-Inspect`) through the web UI, both bot workers, the control-plane PR footer, and outbound HTTP `User-Agent` headers — forks and whitelabel deployments can now rebrand without patching source. - Adds optional `app_icon_url` (custom logo + browser favicon) and `NEXT_PUBLIC_APP_SHORT_NAME` (the historical "Inspect" sidebar label, kept as the default). - New `resolveAppName(env)` helper in `@open-inspect/shared` is the single source of truth — workers resolve at request time, the web bundle inlines `NEXT_PUBLIC_APP_NAME` / `NEXT_PUBLIC_APP_ICON_URL` / `NEXT_PUBLIC_APP_SHORT_NAME` at build time. ## Surfaces wired up | Surface | Source | Variable | |---|---|---| | Web tab title, sign-in heading, landing hero | `metadata`, `sidebar-layout`, `(app)/page` | `NEXT_PUBLIC_APP_NAME` | | Web sidebar logo label | `session-sidebar` | `NEXT_PUBLIC_APP_SHORT_NAME` (defaults to `Inspect`) | | Web sidebar logo image + browser favicon | new `<AppIcon>` helper, `metadata.icons` | `NEXT_PUBLIC_APP_ICON_URL` | | Slack App Home intro text | `slack-bot/src/index.ts` | `APP_NAME` (worker binding) | | Linear OAuth success page | `linear-bot/src/index.ts` | `APP_NAME` | | Linear completion comment headers | `linear-bot/src/callbacks.ts` | `APP_NAME` | | Pull request body footer | `control-plane/src/session/pull-request-service.ts` | `APP_NAME` | | GitHub + GitLab `User-Agent` headers | provider config + `github-app.ts` + `github-bot/github-auth.ts` | `APP_NAME` | Defaults preserve all existing behavior — no diff for deployments that don't set the new vars. ## Backwards compatibility - All new Terraform vars default to current values (`app_name = "Open-Inspect"`, `app_icon_url = ""`). - Sidebar logo defaults to literal `Inspect` (the historical short label) and only follows `APP_NAME` when explicitly overridden. - Function-level User-Agent params on `github-bot` helpers default to `Open-Inspect` so any other callers keep working. ## Screenshots with config values as example **Default** <img width="1390" height="550" alt="CleanShot 2026-05-04 at 17 26 36@2x" src="https://github.com/user-attachments/assets/58ae96c6-e8a8-4e88-a45d-0880786b66b0" /> **With Custom App Name (no Short Name)** <img width="1606" height="460" alt="CleanShot 2026-05-04 at 17 25 59@2x" src="https://github.com/user-attachments/assets/aa204fc0-5be2-47c4-872f-74c9a1d05b76" /> **With Custom App Name and Short Name** <img width="1556" height="496" alt="CleanShot 2026-05-04 at 17 26 14@2x" src="https://github.com/user-attachments/assets/0de8cc8d-6c7d-4813-af8b-906d532a433a" /> ## Docs updated - `docs/GETTING_STARTED.md` — new `terraform.tfvars` block, CI secrets table entries, and a "Customizing the App Name and Icon" section - `docs/SETUP_GUIDE.md` — adds `NEXT_PUBLIC_APP_NAME` / `NEXT_PUBLIC_APP_ICON_URL` to the local dev `.env.local` template 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Optional app branding (name, short name, icon) to customize site title, icon, command menu, sidebar label, PR footers, bot messages, and OAuth success pages. * **Documentation** * Setup and getting-started updated with branding vars, CI/CD secret guidance, and notes about build-time inlining and redeploy steps. * **Tests** * Added coverage ensuring branding appears and is safely escaped in UI, comments, and bot outputs. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: wire APP_NAME, APP_SHORT_NAME, APP_ICON_URL to Terraform (#597) ## Summary Follow-up to #594. The whitelabel branding secrets are documented in `docs/GETTING_STARTED.md` (CI secrets table) but the Terraform workflow doesn't actually read them, so setting `APP_NAME` / `APP_SHORT_NAME` / `APP_ICON_URL` in GitHub Actions secrets currently has no effect on a deployment. This adds the three missing `TF_VAR_` entries to both the **plan** and **apply** jobs in `.github/workflows/terraform.yml`, ordered to match the docs table (right after `GH_BOT_USERNAME`). ## Backwards compatibility - `APP_NAME` uses an `|| 'Open-Inspect'` fallback so the Terraform default (`"Open-Inspect"`) is preserved when the secret isn't set. Without the fallback, GitHub Actions would pass an empty string, which would override the Terraform default. - `APP_SHORT_NAME` and `APP_ICON_URL` are passed bare — their Terraform defaults are already `""`, so an empty string from the unset secret matches the existing default. - Even if an empty string did slip through, the runtime resolvers (`resolveAppName(env)` in workers, `site-config.ts` in the web bundle) already handle empty strings by falling back to `DEFAULT_APP_NAME`. Net effect: no behavior change for users who don't set the new secrets; users who do set them now actually see them flow through to their deployment. ## Test plan - [ ] Open this PR and confirm the **Plan** job runs with the new env vars (visible in the workflow logs) - [ ] After merge, set `APP_NAME` repo secret to a test value and confirm it appears in the next `terraform apply` plan as `var.app_name` - [ ] Confirm a deployment without any of the new secrets still resolves to the `Open-Inspect` defaults end-to-end <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated CI/CD workflow configuration to properly supply environment variables during infrastructure provisioning and deployment steps. <!-- end of auto-generated comment: release notes by coderabbit.ai --> * chore(deps): bump hono from 4.12.14 to 4.12.18 in the npm_and_yarn group across 1 directory (#598) Bumps the npm_and_yarn group with 1 update in the / directory: [hono](https://github.com/honojs/hono). Updates `hono` from 4.12.14 to 4.12.18 <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/honojs/hono/releases">hono's releases</a>.</em></p> <blockquote> <h2>v4.12.18</h2> <h2>Security fixes</h2> <p>This release includes fixes for the following security issues:</p> <h3>Cache Middleware ignores Vary: Authorization / Vary: Cookie leading to cross-user cache leakage</h3> <p>Affects: Cache Middleware. Fixes missing cache-skip handling for <code>Vary: Authorization</code> and <code>Vary: Cookie</code>, where a response cached for one authenticated user could be served to other users. GHSA-p77w-8qqv-26rm</p> <h3>CSS Declaration Injection via Style Object Values in JSX SSR</h3> <p>Affects: hono/jsx. Fixes a missing CSS-context escape for <code>style</code> object values and property names, where untrusted input could inject additional CSS declarations. The impact is limited to CSS and does not allow JavaScript execution. GHSA-qp7p-654g-cw7p</p> <h3>Improper validation of NumericDate claims (exp, nbf, iat) in JWT verify()</h3> <p>Affects: <code>hono/utils/jwt</code>. Fixes improper validation of <code>exp</code>, <code>nbf</code>, and <code>iat</code> claims, where falsy, non-finite, or non-numeric values could silently bypass time-based checks instead of being rejected per RFC 7519. GHSA-hm8q-7f3q-5f36</p> <hr /> <p>Users who use the JWT helper, hono/jsx, or the Cache middleware are strongly encouraged to upgrade to this version.</p> <h2>v4.12.17</h2> <h2>What's Changed</h2> <ul> <li>fix(jsx): normalize SVG attributes on the <!-- raw HTML omitted --> root element by <a href="https://github.com/kfly8"><code>@kfly8</code></a> in <a href="https://redirect.github.com/honojs/hono/pull/4893">honojs/hono#4893</a></li> <li>fix(ssg): add <code>atom+xml</code> and <code>rss+xml</code> to <code>defaultExtensionMap</code> by <a href="https://github.com/yuintei"><code>@yuintei</code></a> in <a href="https://redirect.github.com/honojs/hono/pull/4899">honojs/hono#4899</a></li> <li>fix(cors): make origin optional in CORSOptions by <a href="https://github.com/truffle-dev"><code>@truffle-dev</code></a> in <a href="https://redirect.github.com/honojs/hono/pull/4905">honojs/hono#4905</a></li> <li>fix(types): propagate middleware response types to app.on overloads by <a href="https://github.com/T4ko0522"><code>@T4ko0522</code></a> in <a href="https://redirect.github.com/honojs/hono/pull/4906">honojs/hono#4906</a></li> </ul> <h2>New Contributors</h2> <ul> <li><a href="https://github.com/kfly8"><code>@kfly8</code></a> made their first contribution in <a href="https://redirect.github.com/honojs/hono/pull/4893">honojs/hono#4893</a></li> <li><a href="https://github.com/truffle-dev"><code>@truffle-dev</code></a> made their first contribution in <a href="https://redirect.github.com/honojs/hono/pull/4905">honojs/hono#4905</a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/honojs/hono/compare/v4.12.16...v4.12.17">https://github.com/honojs/hono/compare/v4.12.16...v4.12.17</a></p> <h2>v4.12.16</h2> <h2>Security fixes</h2> <p>This release includes fixes for the following security issues:</p> <h3>Unvalidated JSX Tag Names in hono/jsx May Allow HTML Injection</h3> <p>Affects: hono/jsx. Fixes missing validation of JSX tag names when using <code>jsx()</code> or <code>createElement()</code>, which could allow HTML injection if untrusted input is used as the tag name. GHSA-69xw-7hcm-h432</p> <h3>bodyLimit() can be bypassed for chunked / unknown-length requests</h3> <p>Affects: Body Limit Middleware. Fixes late enforcement for request bodies without a reliable Content-Length (e.g. chunked requests), where oversized requests could reach handlers and return successful responses before being rejected. GHSA-9vqf-7f2p-gf9v</p> <h2>v4.12.15</h2> <h2>What's Changed</h2> <ul> <li>fix(jwt): support single-line PEM keys by <a href="https://github.com/hiendv"><code>@hiendv</code></a> in <a href="https://redirect.github.com/honojs/hono/pull/4889">honojs/hono#4889</a></li> </ul> <!-- raw HTML omitted --> </blockquote> <p>... (truncated)</p> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/honojs/hono/commit/f10dee89ced5956b73c1cdc416d6bc0fd54d63b7"><code>f10dee8</code></a> 4.12.18</li> <li><a href="https://github.com/honojs/hono/commit/a5bd9ebead279ed9d0239ecbd854f629edfc0e57"><code>a5bd9eb</code></a> Merge commit from fork</li> <li><a href="https://github.com/honojs/hono/commit/58d3d3ad5656e007ed99da1b73865975952de5e9"><code>58d3d3a</code></a> Merge commit from fork</li> <li><a href="https://github.com/honojs/hono/commit/568c2ecc1dd556894fad4dfa4a7ba499db6dba9c"><code>568c2ec</code></a> Merge commit from fork</li> <li><a href="https://github.com/honojs/hono/commit/ff2b3d31df1be35f7d597a95dd3369402b6e87f2"><code>ff2b3d3</code></a> 4.12.17</li> <li><a href="https://github.com/honojs/hono/commit/52aaaf9714b06303ce5caa655b1d80675be687e9"><code>52aaaf9</code></a> fix(types): propagate middleware response types to app.on overloads (<a href="https://redirect.github.com/honojs/hono/issues/4906">#4906</a>)</li> <li><a href="https://github.com/honojs/hono/commit/76d5589e9b0569f4e74ec37e8dd6979455f70dfa"><code>76d5589</code></a> fix(cors): make origin optional in CORSOptions (<a href="https://redirect.github.com/honojs/hono/issues/4905">#4905</a>)</li> <li><a href="https://github.com/honojs/hono/commit/8f027e5574e91e3c7f263a728656e3888559e51a"><code>8f027e5</code></a> fix(ssg): add <code>atom+xml</code> and <code>rss+xml</code> to <code>defaultExtensionMap</code> (<a href="https://redirect.github.com/honojs/hono/issues/4899">#4899</a>)</li> <li><a href="https://github.com/honojs/hono/commit/bfba97ca7ea3d4541a3419f1749e5a1a3e8f1727"><code>bfba97c</code></a> fix(jsx): normalize SVG attributes on the <svg> root element (<a href="https://redirect.github.com/honojs/hono/issues/4893">#4893</a>)</li> <li><a href="https://github.com/honojs/hono/commit/90d4182aabd328e2ec6af3f25ec62ddc574ad8cb"><code>90d4182</code></a> 4.12.16</li> <li>Additional commits viewable in <a href="https://github.com/honojs/hono/compare/v4.12.14...v4.12.18">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(terraform): allow overriding R2 media bucket name (#591) Add an optional r2_media_bucket_name variable so the bucket can be pre-created out-of-band and the deployment can point at it instead of having Terraform create one. This unblocks environments where the Cloudflare API token used by Terraform/CI does not have R2 bucket- creation rights and an admin must provision the bucket manually. When the override is empty, behavior is unchanged: the bucket is created as open-inspect-media-<deployment_name>. When set, that exact name is used and operators are expected to terraform import the existing bucket so applies do not try to recreate it. The example tfvars documents manual-setup requirements (same account, private, no CORS/lifecycle, matching location) and clarifies that the Terraform token only needs Workers R2 Storage Read plus Workers Scripts Edit once the bucket exists — object-level R2 permissions are never required because runtime access flows through the in-account MEDIA_BUCKET Worker binding. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Allow specifying a custom R2 media bucket name for production deployments; leaving it empty keeps the automatic default naming. * **Documentation** * Added guidance for overriding the bucket name and for manually pre-creating/importing a bucket, including privacy, location, and import cautions to avoid destructive changes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Cole Murray <colemurray.cs@gmail.com> * chore(deps-dev): bump fast-xml-builder from 1.1.5 to 1.2.0 in the npm_and_yarn group across 1 directory (#602) Bumps the npm_and_yarn group with 1 update in the / directory: [fast-xml-builder](https://github.com/NaturalIntelligence/fast-xml-builder). Updates `fast-xml-builder` from 1.1.5 to 1.2.0 <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/NaturalIntelligence/fast-xml-builder/blob/main/CHANGELOG.md">fast-xml-builder's changelog</a>.</em></p> <blockquote> <p><strong>1.2.0</strong> (2026-05-08)</p> <ul> <li>Add support for <code>sanitizeName</code> option</li> <li>Support xml-naming for validating and sanitizing tag and attribute names</li> </ul> <p><strong>1.1.9</strong> (2026-05-06)</p> <ul> <li>fix: format output for preserve order when indent by is set to empty string</li> </ul> <p><strong>1.1.8</strong> (2026-05-05)</p> <ul> <li>fix: skip text property for PI tags</li> <li>improve typings</li> </ul> <p><strong>1.1.7</strong> (2026--05-04)</p> <ul> <li>fix security issues when attribute value contains quotes</li> </ul> <p><strong>1.1.6</strong> (2026--05-04)</p> <ul> <li>fix security issues related to comment</li> <li>skip comment with null value</li> </ul> <p><strong>1.1.5</strong> (2026-04-17)</p> <ul> <li>fix security issues related to comment and cdata</li> </ul> <p><strong>1.1.4</strong> (2026-03-16)</p> <ul> <li>support maxNestedTags option</li> </ul> <p><strong>1.1.3</strong> (2026-03-13)</p> <ul> <li>declare Matcher & Expression as unknown so user is not forced to install path-expression-matcher</li> </ul> <p><strong>1.1.2</strong> (2026-03-11)</p> <ul> <li>fix typings</li> </ul> <p><strong>1.1.1</strong> (2026-03-11)</p> <ul> <li>upgrade path-expression-matcher to 1.1.3</li> </ul> <p><strong>1.1.0</strong> (2026-03-10)</p> <ul> <li>Integrate <a href="https://github.com/NaturalIntelligence/path-expression-matcher">path-expression-matcher</a></li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/NaturalIntelligence/fast-xml-builder/commit/a9a905b316176ef9a97bdf5450e60efbf0341f25"><code>a9a905b</code></a> for release</li> <li><a href="https://github.com/NaturalIntelligence/fast-xml-builder/commit/42680e8d730c48082268823fd285e10127ddba21"><code>42680e8</code></a> support name sanitization</li> <li><a href="https://github.com/NaturalIntelligence/fast-xml-builder/commit/8b00185bf6be67981ffc40e06c18acbbbe908779"><code>8b00185</code></a> release info</li> <li><a href="https://github.com/NaturalIntelligence/fast-xml-builder/commit/8a08f173d7b9c9a82599fe7de279ca7e12c3ad6b"><code>8a08f17</code></a> allow indentation to be empty string</li> <li><a href="https://github.com/NaturalIntelligence/fast-xml-builder/commit/7fc5decb9613afbd5d03747b1a0f11e0916e34ef"><code>7fc5dec</code></a> update docs</li> <li><a href="https://github.com/NaturalIntelligence/fast-xml-builder/commit/c241b6a8ed1863e5f518490ec1fcc38b13f2c370"><code>c241b6a</code></a> improve documentation</li> <li><a href="https://github.com/NaturalIntelligence/fast-xml-builder/commit/15d5668b53777400c8d80b6e21029c1a70888c78"><code>15d5668</code></a> update for release</li> <li><a href="https://github.com/NaturalIntelligence/fast-xml-builder/commit/98774853a696a1aee4dca830dd3eee2759676bd2"><code>9877485</code></a> fix: skip text property for PI tags</li> <li><a href="https://github.com/NaturalIntelligence/fast-xml-builder/commit/311a2213a817cf31558bea7c0e0807b0d4441814"><code>311a221</code></a> fix <a href="https://redirect.github.com/NaturalIntelligence/fast-xml-builder/issues/5">#5</a> typing import issues</li> <li><a href="https://github.com/NaturalIntelligence/fast-xml-builder/commit/e8fc5b15d9d54b559781961f066de82a55aabcdd"><code>e8fc5b1</code></a> update for releast</li> <li>Additional commits viewable in <a href="https://github.com/NaturalIntelligence/fast-xml-builder/compare/v1.1.5...v1.2.0">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(shared): add Slack API client and mrkdwn sanitizers (#606) ## Summary First PR of the **agent-slack-notify** chain — see [`docs/agent-slack-notification-tool-implementation-plan.md`](./docs/agent-slack-notification-tool-implementation-plan.md), PR 1. - Extracts `slack-bot/src/utils/slack-client.ts` into `@open-inspect/shared/slack/client.ts` so the control-plane can consume the same client for the upcoming agent-slack-notify endpoint. Slack bot token stays a per-call positional argument — no module-level coupling. - Adds pure `mrkdwn` sanitizers (`stripBroadcastMentions`, `sanitizeLinks`, `applyMentionPolicy`, `truncateForSlack`) and the `sanitizeAgentText` composer that PR 4 will run on agent-supplied text before `chat.postMessage`. - Migrates `slack-bot` to import from `@open-inspect/shared`; deletes the local copy. - Replaces `slack-bot`'s ad-hoc `stripMentions` regex with a thin wrapper over `applyMentionPolicy(text, "strip")` plus the existing whitespace cleanup. Six existing `dm-utils` tests pass unchanged. ### Behavior changes The new shared client maps **429**, **non-2xx**, and **malformed-body** responses into the Slack envelope shape (`{ ok: false, error, retryAfter? }`) instead of throwing on `response.json()`. Slack-bot's existing happy-path consumers are unaffected (their tests still pass). PR 4 will rely on the new `retryAfter` field. ### What's intentionally NOT here - No class-based `SlackClient` wrapper — keeps slack-bot's call sites byte-equivalent. Spec §7.1 only requires "client lives in shared, only token differs." - No `listChannels` — the agent-notify path delegates channel access to Slack's bot-membership model and never enumerates channels. ## Test plan - [x] `npm test -w @open-inspect/shared` — 135/135 pass (12 files; +45 new tests across `mrkdwn.test.ts` and `client.test.ts`) - [x] `npm test -w @open-inspect/slack-bot` — 48/48 pass (byte-equivalent migration) - [x] `npm test -w @open-inspect/control-plane` — 1050/1050 pass - [x] `npm test -w @open-inspect/web` / `github-bot` / `linear-bot` — 414/414 pass - [x] `npm run typecheck` — clean across all packages - [x] `npm run lint` and `npm run format -- --check` — clean - [ ] Reviewer sanity-check: confirm `slack-bot`'s reactive completion flow still posts via `postMessage` after the import switch ## Files ``` + packages/shared/src/slack/client.ts (253) + packages/shared/src/slack/client.test.ts (158) + packages/shared/src/slack/mrkdwn.ts ( 72) + packages/shared/src/slack/mrkdwn.test.ts (216) + packages/shared/src/slack/index.ts ( 20) + packages/shared/vitest.config.ts ( 15) ~ packages/shared/src/index.ts ~ packages/slack-bot/src/{index,callbacks,dm-utils}.ts ~ packages/slack-bot/src/utils/resolve-users{,.test}.ts ~ packages/slack-bot/src/index.test.ts - packages/slack-bot/src/utils/slack-client.ts (305) ``` Net: 14 files changed, +755 / −321. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added Slack Web API client utilities for message operations, reaction management, channel/user lookups, and modal views. * Added message sanitization tools for handling mentions, links, and text formatting in Slack messages. * **Tests** * Added comprehensive test coverage for Slack client functions and message sanitization. * **Refactor** * Consolidated Slack utilities to shared package for improved code organization and reusability. [](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/606) <!-- end of auto-generated comment: release notes by coderabbit.ai --> * feat(shared): slack integration settings types + validation (#607) ## Summary PR 2 of the agent-slack-notification feature ([plan](docs/agent-slack-notification-tool-implementation-plan.md)). Defines the typed shape of Slack integration settings and wires it through the `IntegrationSettingsStore` with strict per-level validation. **No behavior change to anything that exists today** — Slack settings are not yet read or used anywhere; that arrives in PRs 3–5. - Adds `"slack"` to `IntegrationId`, `IntegrationSettingsMap`, and `INTEGRATION_DEFINITIONS`. - Defines `SlackRepoSettings` (master switch only) and `SlackGlobalSettings` (master switch + workspace mentions policy). The spec calls out mentions policy as **global only — not overridable per repo**, and the validator enforces it. - Adds a `level: "global" | "repo"` parameter to the existing `IntegrationSettingsStore.validateAndNormalizeSettings` dispatcher and a `validateSlackSettings` private method that: - Rejects unknown fields at both levels - Rejects `mentionsPolicy` at the per-repo level - Rejects non-boolean `agentNotificationsEnabled` - Rejects `mentionsPolicy` values outside `{ "allow", "escape", "strip" }` ## Behavior changes - The integrations list in Settings will now include a "Slack" row. Clicking it routes to a page that currently renders nothing (the `IntegrationDetail` switch in `[id]/page.tsx` falls through to `null`). PR 3 lands the actual UI. - The `/integration-settings/slack/...` API endpoints are now reachable (they go through the existing generic handlers). Operators _could_ curl them today, but there's no UI yet. ## What's intentionally NOT here - **No D1 migration.** `integration_settings` and `integration_repo_settings` already store opaque JSON keyed by `integration_id`; nothing schema-shaped needs to change. - **No web UI.** That's PR 3. - **No control-plane handler / Slack call path.** That's PR 4. - **No defaults applied at storage time.** The plan's "default false / default 'allow'" applies at consumer time (PR 4 will resolve effective config); the store stays consistent with how `GitHubBotSettings` etc. handle defaults today (all fields optional, consumers fill in). ## Naming deviation from spec/plan pseudocode The spec and plan show field names in snake_case (`agent_notifications_enabled`, `mentions_policy`). The codebase universally uses camelCase for settings fields (`autoReviewOnOpen`, `allowedTriggerUsers`, `tunnelPorts`, `terminalEnabled`). I treated the snake_case as pseudocode and used camelCase here to match existing conventions and avoid making this integration the odd one out. ## Type design note `IntegrationEntry<TRepo>` couples the global `defaults` and per-repo settings to a single `TRepo` type. Since `mentionsPolicy` is global-only but `agentNotificationsEnabled` is shared, I used `IntegrationEntry<SlackGlobalSettings>` (the wider type) and enforce the constraint at validation time via the new `level` parameter. `SlackRepoSettings` is exported as documentation of intent — it's the narrower view of what's allowed at the per-repo level, which the validator enforces. ## Test plan - [x] `npm test -w @open-inspect/control-plane` — 1063/1063, including 12 new slack-specific cases (round-trip, accept-all-policies, reject invalid mentionsPolicy, reject non-boolean, reject unknown field at both levels, reject mentionsPolicy at per-repo level, getResolvedConfig merge, mentionsPolicy comes from global only) - [x] `npm test -w @open-inspect/shared` — 135/135 - [x] `npm test -w @open-inspect/slack-bot` — 48/48 (no behavior change) - [x] `npm test -w @open-inspect/web` — 208/208 - [x] `npm test -w @open-inspect/github-bot` — 103/103 - [x] `npm test -w @open-inspect/linear-bot` — 103/103 - [x] `npm run test:integration -w @open-inspect/control-plane` — 327/327 - [x] `npm run typecheck` — clean across all packages - [x] `npm run lint` — clean - [x] `npm run format -- --check` — clean ## Files - `packages/shared/src/types/integrations.ts` (+21/-3) - `packages/control-plane/src/db/integration-settings.ts` (+53/-2) - `packages/control-plane/src/db/integration-settings.test.ts` (+121/-3, including flipping the existing `expect(isValidIntegrationId("slack")).toBe(false)` to `true`) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added Slack integration with configurable global and per-repository settings; options include mentions policy and agent notifications, with per-repo overriding agent notifications. * **Validation** * Enforces allowed fields for global vs repo settings, restricts mentions policy values, and rejects unknown fields. * **Tests** * Added comprehensive Slack settings tests covering CRUD, validation, and resolved-config merge rules. [](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/607) <!-- end of auto-generated comment: release notes by coderabbit.ai --> * feat(web): add Slack integration settings UI (#608) ## Summary PR 3 of the agent-slack-notification feature ([implementation plan](docs/agent-slack-notification-tool-implementation-plan.md)). Operator-facing surface for the Slack integration shipped in #607. Three controls, mirroring the pattern of `github-integration-settings.tsx`: - **Master switch** (global): `agentNotificationsEnabled`, off by default. - **Mentions policy** (global, radio: allow / escape / strip). Workspace-wide; not overridable per repo (per spec §9.1). - **Per-repo override** (`inherit` / `on` / `off`): forces the master switch for a specific repo. Help copy explains that channel access is delegated to Slack bot membership rather than an Open-Inspect-side allowlist (per the v3 plan's allowlist-removal decision). The existing `[id]` API proxy already passes `slack` through to the control plane, so no new web-side API routes are needed. ## What this does NOT do - No control-plane endpoint or sandbox tool yet — those land in PRs 4 and 5. Until then, the UI writes settings the control plane will read once PR 4 wires the feature flag. - No event-list rendering for `slack-notify` tool calls — that's PR 6. ## Test plan - [x] `npm test -w @open-inspect/web` — 217/217 (9 new component tests for `<SlackIntegrationSettings />`) - [x] `npm run typecheck -w @open-inspect/web` - [x] `npm run lint -w @open-inspect/web` - [ ] Manual browser smoke: navigate to Settings → Integrations → Slack, configure settings, save, reload, confirm settings persisted (will verify after this and #607 are deployed together) ## Rollback Revert. Operators can still hit the API directly with curl if needed; PR 4 still works. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added Slack integration settings UI with workspace defaults and per-repository overrides; configure agent notifications and mentions policy; add/remove repo overrides and reset to defaults with confirmation. * **Improvements** * Loading skeleton, help text, and clearer save/reset feedback; repository picker dedupes mixed-case entries. * **Tests** * Expanded test coverage for loading, save/reset flows, per-repo overrides, and server-sync/resync behaviors. [](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/608) <!-- end of auto-generated comment: release notes by coderabbit.ai --> * feat(control-plane): agent-initiated slack-notify endpoint (PR 4) (#609) ## Summary PR 4 of the agent-slack-notification feature ([spec](docs/agent-slack-notification-tool.md), [plan](docs/agent-slack-notification-tool-implementation-plan.md)). Wires the control-plane `POST /sessions/:id/slack-notify` endpoint that sandboxes will call from the new tool in PR 5. - Sandbox-authenticated route validates against the session DO, then loads session metadata + resolved Slack settings from D1. - Sanitizes the agent's text via `@open-inspect/shared/slack/mrkdwn` (broadcast strip → link sanitize → mentions policy → length cap). - Forwards the agent's channel input verbatim to `chat.postMessage`; channel access is delegated to Slack's bot-membership model (no Open-Inspect-side allowlist). - Categorized denial reasons: `feature_unavailable` (no token), `feature_disabled` (master switch off), `empty_message_after_sanitization`, `channel_not_found_or_forbidden`, `rate_limited`, `slack_api_error`. - Emits `tool_call` (completed/error) + `tool_result` events on success and a single failed `tool_call` on denial, with attribution metadata (prompt author, trigger source, parent session, channel input/ID, message ts, permalink, sanitization metadata) — visible in the session transcript. - `SLACK_BOT_TOKEN` added to control-plane `Env` (optional) and bound via Terraform when the variable is non-empty. Bot token never leaves the control plane. - Adds `chat.getPermalink` to `@open-inspect/shared/slack` (was missing) and surfaces `channel` on `chat.postMessage` responses so the success result can include a permalink for the user. The endpoint is reachable but no sandbox can call it yet — the tool itself ships in PR 5. Integration tests exercise it directly via `SELF.fetch`. ## Test plan - [x] `npm test -w @open-inspect/control-plane` — 1078/1078 pass (15 new unit tests for slack-notify). - [x] `npm run test:integration -w @open-inspect/control-plane` — 332/332 pass (5 new workerd integration tests covering 401/403/200/404/channel-passthrough). - [x] `npm test -w @open-inspect/shared` — 135/135 pass (shared client additions). - [x] `npm test -w @open-inspect/slack-bot` — 48/48 pass (additive shared changes don't regress slack-bot). - [x] `npm run typecheck` — clean across all packages. - [x] `npm run lint` — clean. - [ ] Smoke in dev: configure global Slack settings via PR 3's UI, then `curl /sessions/:id/slack-notify` with a synthesized sandbox-auth token; observe message in Slack and tool-call events on the session. - [ ] `grep` dev logs for the bot token after the smoke test — must not appear. ## Notes for review - Channel passthrough is intentional per spec §6.1 — agent input goes to Slack verbatim; Slack does the lookup and rejects forbidden channels with `not_in_channel`/`channel_not_found`/`is_archived`, all mapped to `channel_not_found_or_forbidden`. - Tool events are emitted via the existing `/internal/sandbox-event` DO endpoint with `sandboxId: "control-plane"` to distinguish server-emitted events. The DO's `processSandboxEvent` persists tool_call (status="completed"/"error") and tool_result events normally. - `feature_unavailable` (missing token) is distinct from `feature_disabled` (master switch off) — only the latter emits an event. - Per the plan's open question §1, `reason` is capped at 500 chars and silently truncated server-side. - Per §3, `channel` is capped at 80 chars (Slack's max). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Agents can initiate Slack notifications (threaded messages, formatted blocks, permalinks) with repo-level notification toggle. * **Bug Fixes** * Improved Slack error handling (channel-not-found/forbidden, rate limiting, service/network failures) and rejection of empty/oversized messages after sanitization. * **Tests** * Added unit and integration tests covering success, denial, error, and retry scenarios. * **Chores** * Optional SLACK_BOT_TOKEN env binding added for notifications. [](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/609) <!-- end of auto-generated comment: release notes by coderabbit.ai --> * feat: agent slack-notify sandbox tool + spawn-time gate (PR 5) (#610) ## Summary PR 5 of the agent-slack-notification feature ([spec](docs/agent-slack-notification-tool.md), [plan](docs/agent-slack-notification-tool-implementation-plan.md)). Wires the sandbox-side tool to the endpoint shipped in PR 4 (#609) and enforces the spawn-time gate. - **Sandbox tool**: new `slack-notify.js` plugin in `packages/sandbox-runtime/src/sandbox_runtime/tools/`. Calls `POST /sessions/:id/slack-notify` via the existing bridge client, returns a string suitable for the agent transcript with categorized error hints (e.g. \"ask the user to invite the bot\"). - **Spawn-time gate**: `_install_tools` only copies `slack-notify.js` when `AGENT_SLACK_NOTIFY_ENABLED=\"true\"`. Driven by a new `AGENT_TOOLS_GATED_ON_ENV` map so future tools follow the same pattern. Per spec §7.5, tool presence is fixed for the sandbox's lifetime; per-call authorization is still re-checked against the live master switch by the control-plane endpoint. - **Resolution path**: `SlackAgentNotifyLookup` is wired in the session DO from `IntegrationSettingsStore.getResolvedConfig(\"slack\", repo)` and `env.SLACK_BOT_TOKEN`. The lifecycle manager calls it in `doSpawn()` and `restoreFromSnapshot()`, then threads `agentSlackNotifyEnabled` through `CreateSandboxConfig` → Modal/Daytona providers → `AGENT_SLACK_NOTIFY_ENABLED` env var. Token absence short-circuits to `false` so a misconfigured deployment never installs a tool that would 503 every call. - **Provider plumbing**: Modal client adds `agent_slack_notify_enabled` to both `createSandbox` and `restoreSandbox` JSON bodies. Daytona's `buildEnvVars` adds the env var on the same conditional branch as `CODE_SERVER_PASSWORD`. Modal-infra's `SandboxConfig` adds `agent_slack_notify_enabled: bool = False` and writes `AGENT_SLACK_NOTIFY_ENABLED=\"true\"` only when truthy (matching the existing `TERMINAL_ENABLED` precedent — never written as `\"false\"`). ## Files touched - `packages/sandbox-runtime/src/sandbox_runtime/entrypoint.py` — add `AGENT_TOOLS_GATED_ON_ENV` map and gate in `_install_tools`. - `packages/sandbox-runtime/src/sandbox_runtime/tools/slack-notify.js` — new tool plugin. - `packages/sandbox-runtime/tests/test_tool_installation.py` — add positive case for `AGENT_SLACK_NOTIFY_ENABLED=true`. - `packages/control-plane/src/sandbox/provider.ts` — add `agentSlackNotifyEnabled` to `CreateSandboxConfig` + `RestoreConfig`. - `packages/control-plane/src/sandbox/client.ts` — add field to `CreateSandboxRequest` + `RestoreSandboxRequest` + JSON body. - `packages/control-plane/src/sandbox/providers/modal-provider.ts` — forward field on create + restore. - `packages/control-plane/src/sandbox/providers/daytona-provider.ts` — write `AGENT_SLACK_NOTIFY_ENABLED=\"true\"` when enabled. - `packages/control-plane/src/sandbox/lifecycle/manager.ts` — add `SlackAgentNotifyLookup` interface + `resolveAgentSlackNotifyEnabled()` helper called from `doSpawn` and `restoreFromSnapshot`. - `packages/control-plane/src/session/durable-object.ts` — wire the lookup using `IntegrationSettingsStore` + `env.SLACK_BOT_TOKEN`. - `packages/modal-infra/src/sandbox/manager.py` — add `agent_slack_notify_enabled` to `SandboxConfig`; write env var on truthy in `create_sandbox` + `restore_from_snapshot`. - `packages/modal-infra/src/web_api.py` — accept the field in both `api_create_sandbox` + `api_restore_sandbox`. - `packages/control-plane/src/sandbox/lifecycle/manager.test.ts` — 4 cases for the resolver (true / false / no-lookup / lookup-throws). - `packages/control-plane/src/sandbox/providers/daytona-provider.test.ts` — 2 cases for the env var (set / omitted). - `packages/modal-infra/tests/test_agent_slack_notify_env.py` — 5 cases for create + restore env var passthrough. ## Test plan - [x] `npm run typecheck` — clean across all packages - [x] `npm run lint` — clean - [x] `npm test -w @open-inspect/control-plane` — 1086/1086 unit - [x] `npm run test:integration -w @open-inspect/control-plane` — 332/332 integration - [x] `npm test -w @open-inspect/shared -w @open-inspect/slack-bot` — 184/184 (136 + 48) - [x] `cd packages/sandbox-runtime && uv run pytest` — 270/270 - [x] `cd packages/modal-infra && uv run pytest` — 102/102 - [ ] Manual: spawn a session with the master switch ON in a deployment with `SLACK_BOT_TOKEN` set; confirm `slack-notify.js` lands in `.opencode/tool/`. Spawn another with the switch OFF; confirm the tool is absent. Spawn in a deployment without `SLACK_BOT_TOKEN`; confirm the tool is absent regardless of the master switch. - [ ] Manual: end-to-end agent call → message appears in Slack with attribution and View Session button (the latter requires `WEB_APP_URL` env from PR 4). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a sandbox "slack-notify" tool and a per-repository opt-in for agent Slack notifications (defaults to disabled; lookup errors treated as disabled). * Spawn and restore flows now resolve the repo opt-in and pass it so the runtime sets an env var to install the tool only when enabled. * **Tests** * Added tests validating opt-in resolution, provider payload passthrough, env-var gating, and install/omit behavior. [](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/610) <!-- end of auto-generated comment: release notes by coderabbit.ai --> * fix(control-plane): correct View Session URL in Slack notification (#612) ## Summary - The `View Session` button in the agent slack-notify message was linking to `/sessions/<id>` (plural), but the web app route is `/session/<id>` (singular), so the link 404'd. - One-character fix in `packages/control-plane/src/routes/slack-notify.ts`. ## Test plan - [ ] Trigger a slack-notify from a sandbox agent and confirm the `View Session` button opens the correct session page. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Fixed the "View Session" link in Slack notifications to direct to the correct URL path. [](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/612) <!-- end of auto-generated comment: release notes by coderabbit.ai --> * feat(web): render slack-notify events in session transcript (PR 6) (#611) ## Summary PR 6 / 6 of the agent Slack notification feature ([spec](docs/agent-slack-notification-tool.md), [plan](docs/agent-slack-notification-tool-implementation-plan.md)). Polishes the session transcript so `slack-notify` tool calls render as a first-class event instead of a generic JSON dump. - **Success**: Slack icon, channel input on the line, expanded view with a "View in Slack" permalink (target=_blank, rel=noopener noreferrer) and subtle notes for `truncated` / `strippedBroadcasts` / `mentionsModified`. - **Denial**: error icon plus reason-specific copy: - `channel_not_found_or_forbidden` → "invite the Open-Inspect bot to the channel" hint - `feature_disabled` → "disabled for this repository" - `rate_limited` → retry-window copy - `feature_unavailable`, `empty_message_after_sanitization`, `slack_api_error`, `invalid_input` → tailored headlines - **Unparseable output**: falls back to "No details available" rather than crashing. `ToolCallItem` dispatches early on `event.tool === "slack-notify"` and delegates to the new `SlackNotifyEvent`, so every other tool (Read/Edit/Bash/Apply Patch/etc.) keeps its existing rendering verbatim. The plan referenced files (`event-list.tsx`, `session/` subdir) that don't exist in this repo — the actual transcript rendering goes through `tool-call-item.tsx` + `formatToolCall`. Wiring follows the existing dispatcher pattern instead. ## Files - `packages/web/src/components/slack-notify-event.tsx` (new) — collapsed line + expanded body, success / denial / fallback states - `packages/web/src/components/slack-notify-event.test.tsx` (new) — 7 tests - `packages/web/src/components/tool-call-item.tsx` — early-return delegation when `event.tool === "slack-notify"` - `packages/web/src/components/ui/icons.tsx` — `SlackIcon` ## Test plan - [x] `npm test -w @open-inspect/web` — **228 passed** (7 new in `slack-notify-event.test.tsx`) - [x] `npm run typecheck` — clean across all packages - [x] `npm run lint` — clean - [ ] Manual browser smoke (replay a session with a successful post and a denial — both render correctly) — to be done at feature-merge time per plan §"E2E smoke" ## Verifies - Acceptance gate per plan PR 6: success permalink, denial copy, truncation marker visible, no permalink on denial. - No regression for any other tool (`tool-call-item` only takes the new path for `slack-notify`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added Slack notification event display showing success, permission denied, disabled, and rate-limited states, with contextual messages, optional timestamp, and a Slack icon. * Integrated specialized rendering for Slack notification events in the tool-call list and added expand/collapse with toggle callback. * **Tests** * Added comprehensive tests covering success and error scenarios, external-link safety, flag notes, edge cases, and expansion behavior. [](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/611) <!-- end of auto-generated comment: release notes by coderabbit.ai --> * refactor(shared): consolidate slack-notify contract types (#613) ## Summary - Moves the slack-notify wire-contract types and `DEFAULT_MENTIONS_POLICY` into `@open-inspect/shared/slack/types`. The control-plane route, web event renderer, and Slack settings UI now share one source of truth instead of redeclaring the denial-reason union, success envelope, and default policy in three places. - Adds `resolveSlackSettings(raw)` next to `getResolvedConfig` in the integration-settings DB module, used by both the slack-notify route handler and the Session DO's spawn-time gate closure to replace the ad-hoc `agentNotificationsEnabled === true` and `mentionsPolicy ?? "allow"` checks. - The sandbox JS tool (`packages/sandbox-runtime/.../tools/slack-notify.js`) keeps its own `REASON_GUIDANCE` map: it ships verbatim into the sandbox image, has no access to `@open-inspect/shared` at runtime, and there's no codegen precedent in the repo. Drift mitigation for it is left for a follow-up (e.g. a small symmetry test). ## What moved to `@open-inspect/shared/slack/types` - `SLACK_DENIAL_REASONS` (const tuple) + `SlackDenialReason` type — was a 7-value union in `routes/slack-notify.ts` and a separate 7-value `DENIAL_REASONS` array in `slack-notify-event.tsx` - `SLACK_DENIAL_STATUS` (denial-reason → HTTP status map) — was `STATUS_FOR_REASON` in `routes/slack-notify.ts` - `SlackNotifySuccessOutput` interface — was `SuccessOutput` in the route and `SlackNotifySuccessOutput` in the web component - `SlackNotifyFailureBody` interface — declared but not yet imported anywhere; documents the HTTP wire shape so future callers don't need to reinvent it - `DEFAULT_MENTIONS_POLICY = "allow"` — was a duplicated constant in the route and the settings UI ## What stayed local - `mapSlackError` in `routes/slack-notify.ts` — single TS caller, no dedup benefit yet - `buildBlocks` in `routes/slack-notify.ts` — single TS caller - `DENIAL_COPY` (UI strings) in `slack-notify-event.tsx` — UI-specific copy, not a contract - `SlackAgentNotifyLookup` interface in `sandbox/lifecycle/manager.ts` — single-package consumer - `REASON_GUIDANCE` and `STATUS_FALLBACK_REASON` in the sandbox JS tool — JS runtime can't import from `@open-inspect/shared` ## Drive-by - Removed a duplicated `// Create D1-backed lookups if database is available` comment in `durable-object.ts:703-704`. ## Test plan - [x] `npm run build -w @open-inspect/shared` (rebuilds with the new exports) - [x] `npm run typecheck` — passes across all 6 TS packages - [x] `npm test -w @open-inspect/shared` — 136/136 - [x] `npm test -w @open-inspect/control-plane` — 1095/1095 (+4 new `resolveSlackSettings` unit tests) - [x] `npm run test:integration -w @open-inspect/control-plane` — 332/332 - [x] `npm test -w @open-inspect/web` — 230/230 - [x] `npm test -w @open-inspect/slack-bot` — 48/48 (transitively imports from `@open-inspect/shared`) - [x] ESLint clean on all modified files <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Improvements** * Enhanced Slack integration settings handling with improved default values for missing configurations * Standardized Slack notification error responses with proper HTTP status codes * More consistent behavior for Slack mentions policies across the application [](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/613) <!-- end of auto-generated comment: release notes by coderabbit.ai --> * refactor(shared): SlackEnvelope discriminated union + slack-bot/github-bot polish (#614) ## Summary - Tightens the shared Slack client return types into a discriminated `SlackEnvelope<T>` union so callers narrow once on `.ok` and the success arm carries required fields (channel, ts, user, messages, etc.) instead of optional ones. Removes a class of defensive `?? fallback` and `&& result.x` patterns from callers. - Factors `slackFetch` into typed `slackPost`/`slackGet` helpers; the per-method bodies are now one line each. - Promotes `resolveUserNames` from `slack-bot/utils/` to `@open-inspect/shared/slack/` alongside the rest of the slack client (its only dependency was already `getUserInfo`). - Deletes `slack-bot/utils/internal.ts` and `github-bot/utils/internal.ts` re-export shims; 4 import sites now reach `@open-inspect/shared` directly. - Adds happy-path + error-envelope tests for the 8 previously untested client methods (`getPermalink`, `updateMessage`, `addReaction`, `removeReaction`, `getThreadMessages`, `getUserInfo`, `publishView`, `openView`). Shared package goes from ~138 → 160 unit tests. ## Notes - A few slack-bot call sites needed `.ok` narrowing under the stricter union (`getUserInfo` profile lookup at `index.ts:906`; `ackResult.ts` read at `index.ts:1522`). `slack-notify.ts` no longer needs its `.catch(...)` envelope synthesizer — the client already maps thrown errors to `{ ok: false, error: 'network_error' }`. - No public API removals or behavior changes — same wire interactions, stricter types, fewer lines. ## Test plan - [x] `npm run typecheck` clean across all 6 TS packages - [x] `npm test -w @open-inspect/shared` (160 tests, includes 16 new client tests + 7 moved resolve-users tests) - [x] `npm test -w @open-inspect/slack-bot` (41) - [x] `npm test -w @open-inspect/control-plane` (1095) - [x] `npm run test:integration -w @open-inspect/control-plane` (332/332 — including slack-notify integration coverage) - [x] `npm test -w @open-inspect/github-bot` (103) - [x] `npm test -w @open-inspect/web` (230) - [x] `npm test -w @open-inspect/linear-bot` (103) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **Bug Fixes** * Enhanced Slack API error handling with improved consistency and resilience across messaging, reactions, and user resolution flows. * Strengthened error recovery in authentication and integration handlers to prevent cascading failures. * **Tests** * Expanded test coverage for Slack client functionality including error scenarios and edge cases. [](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/614) <!-- end of auto-generated comment: release notes by coderabbit.ai --> * fix(slack-notify): deduplicate transcript rows by removing synthetic events (#618) ## Summary The slack-notify tool was rendering as **two rows** in the session transcript ("slack-notify · 2 calls"): one from the agent's OpenCode-emitted `tool_call` (output = the plugin's free-form return string, falling through to "No details available") and one from a synthetic `tool_call`/`tool_result` pair injected by the control-plane handler (output = structured JSON). They had different `callId`s, so the frontend dedupe didn't merge them, and `groupEvents` collapsed them under the same tool name with `count = 2`. This PR makes the agent's `tool_call` the single source of truth and aligns slack-notify with the sibling sandbox tools (`spawn-task`, `get-task-status`, `cancel-task`) where the control plane is pure RPC. ### Before ``` slack-notify · 2 calls slack-notify Slack notify #general No details available slack-notify Posted to #general Channel: #general Slack message: View in Slack ``` ### After ``` slack-notify · Posted to #general Channel: #general Slack message: View in Slack ``` ## Architecture - **Control plane** (`routes/slack-notify.ts`) becomes pure RPC. `emitToolEvent`, `emitDenial`, the synthetic `tool_result`, and the `Attribution` interface are gone. Audit attribution moves into `logger.info` (success) and `logger.warn` (denial). `SLACK_BOT_TOKEN`-missing logs at `logger.error` so it surfaces in alerting rather than getting lost in the denial-warning stream. `failureResponse` now takes `SlackWireDenialReason` so the type system enforces "only wire-contract codes go over HTTP." - **Sandbox plugin** (`tools/slack-notify.js`) returns a JSON envelope from `execute()`: - Success: forwards the control plane's response body verbatim (`{ok: true, channelInput, permalink, …}`). - Failure: `{ok: false, reason, agentMessage, retryAfter?}`. `agentMessage` is human guidance for the model; `reason` is the stable code the renderer keys on. New `bridge_error` reason for unreachable control plane. - Defensive: wraps `response.json()` and `response.text()` so the envelope contract holds even on malformed responses; uses `Object.prototype.hasOwnProperty.call` for the reason allowlist; status→reason fallback via lookup table. - **Shared types** (`shared/src/slack/types.ts`): extends `SLACK_DENIAL_REASONS` with `bridge_error` and adds `SlackWireDenialReason = Exclude<SlackDenialReason, "bridge_error">` so `SLACK_DENIAL_STATUS` only includes codes that actually traverse HTTP. Adds `SlackNotifyToolEnvelope` for the plugin contract. - **Renderer** (`slack-notify-event.tsx`): `parseDenialEnvelope` reads `{ok: false, reason}` JSON; `getDenialReason` keeps the legacy bare-code path for events stored in DO storage before this deploy. Surfaces the concrete `retryAfter` value for `rate_limited` (e.g. "Wait 30s before retrying"). ## Test plan - [x] `npm run typecheck` — clean - [x] `npm run lint` — clean - [x] `npm test -w @open-inspect/shared` — 160/160 - [x] `npm test -w @open-inspect/control-plane` — 1096/1096 (no new tests added; existing 18 in `slack-notify.test.ts` rewritten to assert on response shape and audit logs instead of synthetic event emission) - [x] `npm run test:integration -w @open-inspect/control-plane` — 332/332 - [x] `npm test -w @open-inspect/web` — 233/233 (12 in `slack-notify-event.test.tsx`, including new tests for `bridge_error` and concrete `retryAfter` rendering, plus a backward-compat test for legacy `status="error"` events) ## Key behavioral assertions in tests - `expect(sessionFetchMock).not.toHaveBeenCalled()` across all paths confirms the handler never injects events into the DO. - `expect(body).not.toHaveProperty("attribution")` keeps audit data out of the agent-visible response (it goes to logs only). - The integration test queries DO storage for any slack-notify-related event row and asserts none exist after a successful post. ## Notes / follow-ups Three items from review explicitly deferred: 1. **Audit-log gap on `parseBody` failures.** Pre-existing; the `invalid_input` paths (oversized text, missing channel, malformed JSON) bypass `logDenial`. Worth closing as a separate audit-coverage PR. 2. **Plugin unit test.** Sibling tools (`spawn-task`, `get-task-status`) have none either; would require new test infra. `REASON_GUIDANCE`/`STATUS_FALLBACK_REASON` symmetry with shared types is mentioned in the plugin's JSDoc and is the suggested target for a small symmetry test. 3. **Reason-set drift.** The plugin's `REASON_GUIDANCE` keys must stay symmetric with `SLACK_DENIAL_REASONS` in shared by hand (the JS plugin can't import from `@open-inspect/shared` at runtime). Same constraint #613 documented; no codegen precedent yet. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Slack tool and UI now use a stable JSON success/failure envelope; UI shows retry countdown and a new connectivity (“bridge”) message. * **Bug Fixes** * Consistent denial and network failure codes, clearer logging, and removal of attribution from agent-visible responses. * **Tests** * Tests updated to assert HTTP response envelopes and console/audit logs; integration tests verify no unexpected persisted events. [](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/618) <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Cole Murray <colemurray.cs@gmail.com> Co-authored-by: Donn Felker <donn@donnfelker.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Summary
PR 2 of the agent-slack-notification feature (plan). Defines the typed shape of Slack integration settings and wires it through the
IntegrationSettingsStorewith strict per-level validation. No behavior change to anything that exists today — Slack settings are not yet read or used anywhere; that arrives in PRs 3–5."slack"toIntegrationId,IntegrationSettingsMap, andINTEGRATION_DEFINITIONS.SlackRepoSettings(master switch only) andSlackGlobalSettings(master switch + workspace mentions policy). The spec calls out mentions policy as global only — not overridable per repo, and the validator enforces it.level: "global" | "repo"parameter to the existingIntegrationSettingsStore.validateAndNormalizeSettingsdispatcher and avalidateSlackSettingsprivate method that:mentionsPolicyat the per-repo levelagentNotificationsEnabledmentionsPolicyvalues outside{ "allow", "escape", "strip" }Behavior changes
IntegrationDetailswitch in[id]/page.tsxfalls through tonull). PR 3 lands the actual UI./integration-settings/slack/...API endpoints are now reachable (they go through the existing generic handlers). Operators could curl them today, but there's no UI yet.What's intentionally NOT here
integration_settingsandintegration_repo_settingsalready store opaque JSON keyed byintegration_id; nothing schema-shaped needs to change.GitHubBotSettingsetc. handle defaults today (all fields optional, consumers fill in).Naming deviation from spec/plan pseudocode
The spec and plan show field names in snake_case (
agent_notifications_enabled,mentions_policy). The codebase universally uses camelCase for settings fields (autoReviewOnOpen,allowedTriggerUsers,tunnelPorts,terminalEnabled). I treated the snake_case as pseudocode and used camelCase here to match existing conventions and avoid making this integration the odd one out.Type design note
IntegrationEntry<TRepo>couples the globaldefaultsand per-repo settings to a singleTRepotype. SincementionsPolicyis global-only butagentNotificationsEnabledis shared, I usedIntegrationEntry<SlackGlobalSettings>(the wider type) and enforce the constraint at validation time via the newlevelparameter.SlackRepoSettingsis exported as documentation of intent — it's the narrower view of what's allowed at the per-repo level, which the validator enforces.Test plan
npm test -w @open-inspect/control-plane— 1063/1063, including 12 new slack-specific cases (round-trip, accept-all-policies, reject invalid mentionsPolicy, reject non-boolean, reject unknown field at both levels, reject mentionsPolicy at per-repo level, getResolvedConfig merge, mentionsPolicy comes from global only)npm test -w @open-inspect/shared— 135/135npm test -w @open-inspect/slack-bot— 48/48 (no behavior change)npm test -w @open-inspect/web— 208/208npm test -w @open-inspect/github-bot— 103/103npm test -w @open-inspect/linear-bot— 103/103npm run test:integration -w @open-inspect/control-plane— 327/327npm run typecheck— clean across all packagesnpm run lint— cleannpm run format -- --check— cleanFiles
packages/shared/src/types/integrations.ts(+21/-3)packages/control-plane/src/db/integration-settings.ts(+53/-2)packages/control-plane/src/db/integration-settings.test.ts(+121/-3, including flipping the existingexpect(isValidIntegrationId("slack")).toBe(false)totrue)Summary by CodeRabbit