feat: agent slack-notify sandbox tool + spawn-time gate (PR 5)#610
Merged
Conversation
The control plane resolves `master_switch && SLACK_BOT_TOKEN_present` at spawn time and threads it through CreateSandboxConfig → Modal/Daytona providers → AGENT_SLACK_NOTIFY_ENABLED env var. The sandbox-runtime entrypoint reads that env at boot and only copies `slack-notify.js` into `.opencode/tool/` when the gate is on, matching spec §7.5: tool presence is fixed per spawn; per-call authorization is still re-checked against the live master switch by the control-plane endpoint shipped in PR 4. Spec / plan: docs/agent-slack-notification-tool.md, docs/agent-slack-notification-tool-implementation-plan.md (PR 5).
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an optional spawn-time flag that gates a new slack-notify sandbox tool. The flag threads from client DTOs through lifecycle manager (with per-repo lookup), into providers and modal infra, and into the runtime where tool installation is conditional. ChangesAgent Slack Notification Feature
Sequence Diagram(s)sequenceDiagram
participant Client
participant ControlPlane as LifecycleManager
participant Provider
participant Runtime
Client->>ControlPlane: POST create/restore with agent_slack_notify_enabled
ControlPlane->>ControlPlane: resolveAgentSlackNotifyEnabled(repo)
alt lookup true
ControlPlane->>Provider: createSandbox(config with agentSlackNotifyEnabled=true)
else lookup false
ControlPlane->>Provider: createSandbox(config with agentSlackNotifyEnabled=false)
end
Provider->>Runtime: set env / restore snapshot (may include AGENT_SLACK_NOTIFY_ENABLED)
Runtime->>Runtime: _install_tools checks AGENT_SLACK_NOTIFY_ENABLED and installs slack-notify.js only if "true"
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Terraform Validation Results
Pushed by: @ColeMurray, Action: |
![open-inspect[bot]](https://avatars.githubusercontent.com/u/2492022?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Summary
Reviewed PR #610, feat: agent slack-notify sandbox tool + spawn-time gate, by @ColeMurray. 14 files changed, +484/-3. The end-to-end wiring for the spawn-time gate and sandbox/provider propagation looks correct, and the added tests cover the key happy-path and failure-path behavior well.
Critical Issues
None.
Suggestions
- [Code Quality]
packages/sandbox-runtime/src/sandbox_runtime/tools/slack-notify.js:66- The 404 handling currently tells the agent to ask the user to invite the bot, but the server intentionally uses the same 404 forchannel_not_found,not_in_channel, andis_archived. A softer hint would avoid steering the agent toward the wrong remediation for archived channels or typos.
Nitpicks
None.
Positive Feedback
- The spawn-time gate is threaded cleanly from session config through both Modal and Daytona paths without leaking the Slack token into the sandbox.
- Falling back to
falseon lookup failure is a good safety default and keeps sandbox creation resilient. - Test coverage is strong across control-plane, sandbox-runtime, and modal-infra, including negative/error cases.
Questions
None.
Verdict
Approve: Ready to merge, no blocking issues from this review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/sandbox-runtime/src/sandbox_runtime/tools/slack-notify.js (1)
56-75: 💤 Low valueConsider explicit handling for 401 Unauthorized.
The error mapping covers the main failure modes well. However, if the control-plane endpoint can return 401 (e.g., sandbox auth token expired), it might be worth adding an explicit case to guide the agent appropriately, rather than falling through to the generic message.
💡 Optional: Add 401 handling
if (response.status === 429) { return `Rate limited by Slack: ${errorMessage}. Wait before retrying.`; } + if (response.status === 401) { + return `Cannot post to Slack: ${errorMessage}. Session authentication failed — the sandbox may need to restart.`; + } return `Failed to post to Slack: ${errorMessage} (HTTP ${response.status})`;🤖 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/sandbox-runtime/src/sandbox_runtime/tools/slack-notify.js` around lines 56 - 75, Add an explicit 401 branch inside the existing error handling block that checks "if (!response.ok)" so 401 Unauthorized is handled separately; use the already-extracted errorMessage (from extractError) and return a clear message e.g. "Unauthorized: {errorMessage}. The sandbox auth token may have expired — re-authenticate or refresh the token." Reference the same response.status check pattern used for 503/403/404/etc. and keep args.channel/context usage consistent with the existing messages.packages/control-plane/src/sandbox/providers/daytona-provider.test.ts (1)
324-336: ⚡ Quick winAdd explicit
agentSlackNotifyEnabled: falsecoverage.Line 332 currently validates the “absent key” path, but runtime disabled behavior comes through as explicit
false. Add that case so regressions don’t slip.Proposed test addition
+ it("omits AGENT_SLACK_NOTIFY_ENABLED when explicitly false", async () => { + const client = createMockClient(); + const provider = new DaytonaSandboxProvider( + client, + defaultProviderConfig, + defaultGetCloneToken + ); + + await provider.createSandbox({ ...baseCreateConfig, agentSlackNotifyEnabled: false }); + + const envVars = (client.createSandbox as ReturnType<typeof vi.fn>).mock.calls[0][0].env; + expect(envVars.AGENT_SLACK_NOTIFY_ENABLED).toBeUndefined(); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/control-plane/src/sandbox/providers/daytona-provider.test.ts` around lines 324 - 336, Add a new unit test that covers the explicit disabled case by calling provider.createSandbox with a config that sets agentSlackNotifyEnabled: false (use DaytonaSandboxProvider, baseCreateConfig and the same createMockClient setup as the existing test), then inspect the mock call to client.createSandbox and assert that env.AGENT_SLACK_NOTIFY_ENABLED is undefined; this mirrors the existing "absent key" test but supplies agentSlackNotifyEnabled: false to prevent regressions.packages/control-plane/src/sandbox/lifecycle/manager.test.ts (1)
1681-1753: ⚡ Quick winAdd slack-gate assertions for snapshot-restore path too.
This suite covers
createSandbox, but notrestoreFromSnapshot; gate propagation should be locked in on both paths.Suggested test shape
+ it("passes agentSlackNotifyEnabled=true on restore when lookup returns true", async () => { + const lookup: SlackAgentNotifyLookup = { isEnabledForRepo: vi.fn(async () => true) }; + const sandbox = createMockSandbox({ + status: "stopped", + snapshot_image_id: "img-abc123", + }); + const storage = createMockStorage(createMockSession(), sandbox); + const provider = createMockProvider(); + const manager = new SandboxLifecycleManager( + provider, + storage, + createMockBroadcaster(), + createMockWebSocketManager(false), + createMockAlarmScheduler(), + createMockIdGenerator(), + { ...createTestConfig(), slackAgentNotifyLookup: lookup } + ); + + await manager.spawnSandbox(); + + expect(provider.restoreFromSnapshot).toHaveBeenCalledWith( + expect.objectContaining({ agentSlackNotifyEnabled: true }) + ); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/control-plane/src/sandbox/lifecycle/manager.test.ts` around lines 1681 - 1753, Add equivalent tests covering the snapshot-restore path by calling SandboxLifecycleManager.restoreFromSnapshot and asserting the slack gate is propagated the same way as spawnSandbox; for each case in this suite (lookup true, lookup false, no lookup, lookup throws) call manager.restoreFromSnapshot(...) and assert the provider method used for restores (e.g., provider.restoreSandbox or provider.createSandbox used by restoreFromSnapshot) is invoked with expect.objectContaining({ agentSlackNotifyEnabled: true/false }) as appropriate, reusing buildManagerWith to construct the manager and lookup mocks and matching the same assertion patterns used for spawnSandbox.
🤖 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/control-plane/src/sandbox/lifecycle/manager.test.ts`:
- Around line 1681-1753: Add equivalent tests covering the snapshot-restore path
by calling SandboxLifecycleManager.restoreFromSnapshot and asserting the slack
gate is propagated the same way as spawnSandbox; for each case in this suite
(lookup true, lookup false, no lookup, lookup throws) call
manager.restoreFromSnapshot(...) and assert the provider method used for
restores (e.g., provider.restoreSandbox or provider.createSandbox used by
restoreFromSnapshot) is invoked with expect.objectContaining({
agentSlackNotifyEnabled: true/false }) as appropriate, reusing buildManagerWith
to construct the manager and lookup mocks and matching the same assertion
patterns used for spawnSandbox.
In `@packages/control-plane/src/sandbox/providers/daytona-provider.test.ts`:
- Around line 324-336: Add a new unit test that covers the explicit disabled
case by calling provider.createSandbox with a config that sets
agentSlackNotifyEnabled: false (use DaytonaSandboxProvider, baseCreateConfig and
the same createMockClient setup as the existing test), then inspect the mock
call to client.createSandbox and assert that env.AGENT_SLACK_NOTIFY_ENABLED is
undefined; this mirrors the existing "absent key" test but supplies
agentSlackNotifyEnabled: false to prevent regressions.
In `@packages/sandbox-runtime/src/sandbox_runtime/tools/slack-notify.js`:
- Around line 56-75: Add an explicit 401 branch inside the existing error
handling block that checks "if (!response.ok)" so 401 Unauthorized is handled
separately; use the already-extracted errorMessage (from extractError) and
return a clear message e.g. "Unauthorized: {errorMessage}. The sandbox auth
token may have expired — re-authenticate or refresh the token." Reference the
same response.status check pattern used for 503/403/404/etc. and keep
args.channel/context usage consistent with the existing messages.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2888e737-a96f-4c68-b1eb-e9bf786881a3
📒 Files selected for processing (14)
packages/control-plane/src/sandbox/client.tspackages/control-plane/src/sandbox/lifecycle/manager.test.tspackages/control-plane/src/sandbox/lifecycle/manager.tspackages/control-plane/src/sandbox/provider.tspackages/control-plane/src/sandbox/providers/daytona-provider.test.tspackages/control-plane/src/sandbox/providers/daytona-provider.tspackages/control-plane/src/sandbox/providers/modal-provider.tspackages/control-plane/src/session/durable-object.tspackages/modal-infra/src/sandbox/manager.pypackages/modal-infra/src/web_api.pypackages/modal-infra/tests/test_agent_slack_notify_env.pypackages/sandbox-runtime/src/sandbox_runtime/entrypoint.pypackages/sandbox-runtime/src/sandbox_runtime/tools/slack-notify.jspackages/sandbox-runtime/tests/test_tool_installation.py
The control-plane endpoint maps channel_not_found, not_in_channel, and is_archived all to the same 404 (channel_not_found_or_forbidden), so the prior message wrongly steered the agent to "invite the bot" for typos and archived channels. Reword to cover all three causes and present inviting the bot as one possible fix.
Terraform Validation Results
Pushed by: @ColeMurray, Action: |
Ruff SIM117 flagged the nested `patch.dict` + `_patch_paths` block in test_install_tools_includes_slack_notify_when_enabled. Use Python 3.10 parenthesised-context syntax to combine them.
Terraform Validation Results
Pushed by: @ColeMurray, Action: |
Adds two regression-detection tests requested in PR review: - daytona-provider: explicit `agentSlackNotifyEnabled: false` mirrors the absent-key case to lock in "never write 'false' as a string". - lifecycle manager: mirrors the four spawn-path slack-gate cases against the snapshot-restore branch so a regression in either resolution call site is caught.
Terraform Validation Results
Pushed by: @ColeMurray, Action: |
Same fact ("spawn-time decision, fixed for sandbox lifetime, ...") was
re-stated at every layer it could attach to. Pulled it down to one
canonical anchor on CreateSandboxConfig and short cross-references
elsewhere. Dropped negative-space rationale in tests, the "503 on every
call" rationale is the one piece of non-obvious WHY worth keeping.
No behavior change; comments only.
Terraform Validation Results
Pushed by: @ColeMurray, Action: |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/control-plane/src/session/durable-object.ts (1)
714-730: ⚡ Quick winConsider adding error handling to prevent spawn failures.
The implementation correctly follows the token-absence defensive pattern, but if
getResolvedConfigthrows an error (e.g., database query failure, malformed data), it will propagate and potentially fail sandbox spawning. Given the comment states the goal is to "never install a tool that would 503 on every call," consider adding try-catch to returnfalseon any error.🛡️ Proposed defensive error handling
slackAgentNotifyLookup = { isEnabledForRepo: async (repoOwner, repoName) => { if (!tokenPresent) return false; - const { settings } = await settingsStore.getResolvedConfig( - "slack", - `${repoOwner}/${repoName}` - ); - return settings.agentNotificationsEnabled === true; + try { + const { settings } = await settingsStore.getResolvedConfig( + "slack", + `${repoOwner}/${repoName}` + ); + return settings?.agentNotificationsEnabled === true; + } catch (error) { + // Fail closed: don't install tool if config lookup fails + return false; + } }, };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/control-plane/src/session/durable-object.ts` around lines 714 - 730, The isEnabledForRepo implementation for slackAgentNotifyLookup should guard against exceptions from IntegrationSettingsStore.getResolvedConfig; wrap the await settingsStore.getResolvedConfig(...) call in a try-catch inside the isEnabledForRepo async function (checking tokenPresent first) and return false on any caught error (optionally log the error via this.env or a logger), so failures in getResolvedConfig do not propagate and cause sandbox/spawn failures.
🤖 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/control-plane/src/session/durable-object.ts`:
- Around line 714-730: The isEnabledForRepo implementation for
slackAgentNotifyLookup should guard against exceptions from
IntegrationSettingsStore.getResolvedConfig; wrap the await
settingsStore.getResolvedConfig(...) call in a try-catch inside the
isEnabledForRepo async function (checking tokenPresent first) and return false
on any caught error (optionally log the error via this.env or a logger), so
failures in getResolvedConfig do not propagate and cause sandbox/spawn failures.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2add4bba-92b0-4939-994f-6fe490525769
📒 Files selected for processing (7)
packages/control-plane/src/sandbox/lifecycle/manager.tspackages/control-plane/src/sandbox/provider.tspackages/control-plane/src/session/durable-object.tspackages/modal-infra/src/sandbox/manager.pypackages/sandbox-runtime/src/sandbox_runtime/entrypoint.pypackages/sandbox-runtime/src/sandbox_runtime/tools/slack-notify.jspackages/sandbox-runtime/tests/test_tool_installation.py
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/sandbox-runtime/tests/test_tool_installation.py
- packages/sandbox-runtime/src/sandbox_runtime/entrypoint.py
- packages/sandbox-runtime/src/sandbox_runtime/tools/slack-notify.js
- packages/modal-infra/src/sandbox/manager.py
- packages/control-plane/src/sandbox/lifecycle/manager.ts
This was referenced May 10, 2026
hreiten
added a commit
to watchdog-no/background-agents
that referenced
this pull request
May 11, 2026
…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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR 5 of the agent-slack-notification feature (spec, plan). Wires the sandbox-side tool to the endpoint shipped in PR 4 (#609) and enforces the spawn-time gate.
slack-notify.jsplugin inpackages/sandbox-runtime/src/sandbox_runtime/tools/. CallsPOST /sessions/:id/slack-notifyvia 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")._install_toolsonly copiesslack-notify.jswhenAGENT_SLACK_NOTIFY_ENABLED=\"true\". Driven by a newAGENT_TOOLS_GATED_ON_ENVmap 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.SlackAgentNotifyLookupis wired in the session DO fromIntegrationSettingsStore.getResolvedConfig(\"slack\", repo)andenv.SLACK_BOT_TOKEN. The lifecycle manager calls it indoSpawn()andrestoreFromSnapshot(), then threadsagentSlackNotifyEnabledthroughCreateSandboxConfig→ Modal/Daytona providers →AGENT_SLACK_NOTIFY_ENABLEDenv var. Token absence short-circuits tofalseso a misconfigured deployment never installs a tool that would 503 every call.agent_slack_notify_enabledto bothcreateSandboxandrestoreSandboxJSON bodies. Daytona'sbuildEnvVarsadds the env var on the same conditional branch asCODE_SERVER_PASSWORD. Modal-infra'sSandboxConfigaddsagent_slack_notify_enabled: bool = Falseand writesAGENT_SLACK_NOTIFY_ENABLED=\"true\"only when truthy (matching the existingTERMINAL_ENABLEDprecedent — never written as\"false\").Files touched
packages/sandbox-runtime/src/sandbox_runtime/entrypoint.py— addAGENT_TOOLS_GATED_ON_ENVmap 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 forAGENT_SLACK_NOTIFY_ENABLED=true.packages/control-plane/src/sandbox/provider.ts— addagentSlackNotifyEnabledtoCreateSandboxConfig+RestoreConfig.packages/control-plane/src/sandbox/client.ts— add field toCreateSandboxRequest+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— writeAGENT_SLACK_NOTIFY_ENABLED=\"true\"when enabled.packages/control-plane/src/sandbox/lifecycle/manager.ts— addSlackAgentNotifyLookupinterface +resolveAgentSlackNotifyEnabled()helper called fromdoSpawnandrestoreFromSnapshot.packages/control-plane/src/session/durable-object.ts— wire the lookup usingIntegrationSettingsStore+env.SLACK_BOT_TOKEN.packages/modal-infra/src/sandbox/manager.py— addagent_slack_notify_enabledtoSandboxConfig; write env var on truthy increate_sandbox+restore_from_snapshot.packages/modal-infra/src/web_api.py— accept the field in bothapi_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
npm run typecheck— clean across all packagesnpm run lint— cleannpm test -w @open-inspect/control-plane— 1086/1086 unitnpm run test:integration -w @open-inspect/control-plane— 332/332 integrationnpm test -w @open-inspect/shared -w @open-inspect/slack-bot— 184/184 (136 + 48)cd packages/sandbox-runtime && uv run pytest— 270/270cd packages/modal-infra && uv run pytest— 102/102SLACK_BOT_TOKENset; confirmslack-notify.jslands in.opencode/tool/. Spawn another with the switch OFF; confirm the tool is absent. Spawn in a deployment withoutSLACK_BOT_TOKEN; confirm the tool is absent regardless of the master switch.WEB_APP_URLenv from PR 4).Summary by CodeRabbit
New Features
Tests