fix: type and centralize Modal sandbox HTTP boundaries by open-inspect[bot] · Pull Request #1631 · ColeMurray/background-agents

open-inspect · 2026-08-27T18:24:47Z

Summary

add strict Pydantic request models for interactive sandbox create and snapshot restore
validate repository owner/name pairs and nested multi-repository identities at the HTTP boundary
parse create/restore requests once and construct manager inputs from typed values
centralize authentication, timing, HTTP exception tracking, generic exception mapping, and modal.http_request logging in a small async context manager
map unexpected internal failures to sanitized HTTP 500 responses instead of HTTP 200 { success: false } payloads
preserve explicit build-session not-found handling and all endpoint-specific success response shapes

Compatibility

The existing rolling-deployment policy is preserved independently from strict field typing:

unknown top-level request fields remain ignored through _ModalRequestModel (extra="ignore")
unknown nested restore session_config fields remain preserved (extra="allow") so snapshots can round-trip fields introduced by newer control-plane deployments
known fields use strict types, so values such as "false" are rejected rather than coerced to truthy booleans
optional no-repository sessions remain supported, while partial repository identities are rejected
default timeout and VNC behavior, repo-image create behavior, snapshot clone-token compatibility, environment variables, settings, code-server/VNC/Slack flags, multi-repository session configuration, and structured correlation IDs are preserved

No control-plane changes were necessary. Its Modal client already handles non-2xx responses explicitly, and successful response payloads are unchanged.

Error Envelope

The shared endpoint execution seam owns:

bearer authentication before request and control-plane URL validation
request timing and success/error outcome tracking
propagation of known HTTPException status/detail values
logging unexpected exceptions server-side and mapping them to bounded 500 Internal server error responses
final modal.http_request logging, including endpoint-specific trace/request/session/sandbox/build identifiers

Control-plane URL validation no longer reflects the submitted URL in client-visible errors.

TDD Evidence

Red:

added focused tests before production changes
initial focused run: 11 failed, 30 passed
expected failures showed string booleans being accepted, malformed typed fields reaching Modal/domain code, and generic create/restore failures returning normally instead of raising HTTP 500

Green:

added the create/restore request models and applied the minimal execution seam to those handlers
focused create/restore run: 41 passed

Refactor:

extracted all remaining authenticated endpoint envelopes onto the tested seam
combined focused create/build API run after extraction: 74 passed
applied the code-simplifier review and removed only redundant execution-path state and an unreachable error mapping
reran focused and full verification after refactoring

Verification

uv run pytest tests/test_web_api_create_sandbox.py tests/test_web_api_build_sandbox.py -q -> 74 passed
uv run pytest tests/ -q -> 210 passed
uv run ruff check src/web_api.py tests/test_web_api_create_sandbox.py -> passed
uv run ruff format --check src/web_api.py tests/test_web_api_create_sandbox.py -> passed
git diff --check -> passed

An additional uv run mypy src/web_api.py was attempted and reports 16 existing strict-typing issues in this legacy module, primarily pre-existing unparameterized endpoint dict annotations and dynamically re-exported constants. This check is not part of the requested Modal validation set and no new mypy-specific scope was added.

Risks

malformed create/restore payloads that previously reached domain code or were silently coerced now receive HTTP 400 errors
unexpected failures now correctly produce non-2xx responses; callers relying on the erroneous HTTP-200 error object behavior will observe the corrected contract
unknown-field handling remains intentionally permissive for rolling deployments as described above

Scope

This change is limited to audit finding A21. It does not include A22's SandboxProvider capability/launch-contract refactor, provider adapter consolidation, or image-build lifecycle changes.

Created with Open-Inspect

github-actions · 2026-08-27T18:25:16Z

Terraform Validation Results

Step	Status
Format	✅
Init	✅
Validate	✅
Tests	✅

Note: Terraform plan was skipped because secrets are not configured. This is expected for external contributors. See docs/GETTING_STARTED.md for setup instructions.

Pushed by: @open-inspect[bot], Action: pull_request

open-inspect

[deep review] Four blockers found: the create path duplicates and loses future canonical SessionConfig fields, restore strips future nested repository fields despite its compatibility contract, the shared execution wrapper logs cancelled requests as successful 200s, and required callback URL/token inputs remain optional empty strings. Inline comments include focused structural remedies. This account authored the PR, so GitHub does not permit a changes-requested review.

github-actions · 2026-08-27T18:33:25Z

Terraform Validation Results

Step	Status
Format	✅
Init	✅
Validate	✅
Tests	✅

Note: Terraform plan was skipped because secrets are not configured. This is expected for external contributors. See docs/GETTING_STARTED.md for setup instructions.

Pushed by: @open-inspect[bot], Action: pull_request

github-actions · 2026-08-27T18:36:10Z

Terraform Validation Results

Step	Status
Format	✅
Init	✅
Validate	✅
Tests	✅

Note: Terraform plan was skipped because secrets are not configured. This is expected for external contributors. See docs/GETTING_STARTED.md for setup instructions.

Pushed by: @open-inspect[bot], Action: pull_request

github-actions · 2026-08-27T18:38:59Z

Terraform Validation Results

Step	Status
Format	✅
Init	✅
Validate	✅
Tests	✅

Note: Terraform plan was skipped because secrets are not configured. This is expected for external contributors. See docs/GETTING_STARTED.md for setup instructions.

Pushed by: @open-inspect[bot], Action: pull_request

github-actions · 2026-08-27T18:41:39Z

Terraform Validation Results

Step	Status
Format	✅
Init	✅
Validate	✅
Tests	✅

Note: Terraform plan was skipped because secrets are not configured. This is expected for external contributors. See docs/GETTING_STARTED.md for setup instructions.

Pushed by: @open-inspect[bot], Action: pull_request

* chore(control-plane): define DEFAULT_BASE_BRANCH once (#1617) ## Summary Closes out the deps-style normalization campaign (#1608/#1609/#1612/#1615/#1616): the last-resort `"main"` base-branch fallback was written as a literal at seven independent sites. Per the repo convention ("define each default value exactly once — extract to a named constant and import everywhere"), it is now `DEFAULT_BASE_BRANCH` in `src/repos/default-branch.ts`, imported at all seven. Deferred from the #1608 review round. ## The seven sites All express the same concept — the branch assumed only when neither the caller nor the SCM provider's repository metadata supplies one; configured per-repo defaults (#757) always win: - `repos/resolve.ts` — `input.baseBranch?.trim() || access.defaultBranch || …` - `automation/repository.ts` — same shape for automation repo selections - `routes/session-child-spawn.ts` — spawn-context fallback - `session/initialize.ts` and `session/http/handlers/session-lifecycle.handler.ts` — init-payload fallback - `session/snapshot-reader.ts` and `session/sandbox-lifecycle-adapters.ts` — legacy repository rows persisted before `base_branch` was stored Test fixtures keep their literals (they are inputs, not the default's definition). No behavior change: the constant's value is `"main"`. ## Testing - `npm run typecheck` (all three programs) clean; ESLint clean - Unit + integration batteries green - `rg '\?\? "main"|\|\| "main"' src` (non-test) → no matches  ## Summary by CodeRabbit * **Bug Fixes** * Standardized repository branch fallback behavior across session initialization, automation, repository resolution, and child sessions. * Repositories without a configured or provider-supplied base branch now consistently use the default `main` branch.  * fix(control-plane): show automation children in Mine (#1619) ## Summary - keep directly automated and GitHub bot sessions hidden from the Mine inbox - allow user-attributed agent children with automation lineage to appear as re-rooted Mine entries - add integration coverage for an automation root with a user-attributed child ## Root cause The Mine inbox rejected every session with a non-null `automation_id`. Child sessions inherit that ID from an automation parent, so even children created after a user follow-up were filtered out. ## Verification - `npm run test:integration -w @open-inspect/control-plane -- session-inbox.test.ts` - `npm test -w @open-inspect/control-plane -- src/routes/session-index.test.ts src/db/session-index.test.ts` - `npm run typecheck -w @open-inspect/control-plane` - `npm run lint -w @open-inspect/control-plane` - focused Prettier check - `git diff --check` --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/115a7540a10e9695039d22afac46028d)*  ## Summary by CodeRabbit * **New Features** * Updated the “Mine” inbox view to include agent sessions spawned from automated sessions. * Clarified the option used to exclude automated sessions. * **Bug Fixes** * Improved inbox filtering so directly automated and GitHub Bot sessions are excluded while eligible child sessions remain visible. * **Tests** * Expanded integration coverage for automated sessions, their child sessions, and user-owned sessions in the “Mine” view.  --------- Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com> * Add human PR feedback Autofix (#1182) ## Summary - queues eligible GitHub PR comments and submitted reviews after signed webhook validation - re-reads authoritative GitHub state, correlates the owning session, and applies repository policy - records durable decisions and atomically admits one idempotent message into the existing SessionDO queue - enforces the rolling per-PR attempt cap and recovers ambiguous or duplicate deliveries - keeps Autofix default-off and preserves explicit mention behavior - uses D1 migration 0058 without colliding with current main ## Stack 1. This PR: human and explicitly allowlisted review feedback foundation 2. #1183: producer-agnostic Open Inspect App reviews 3. #1184: configuration, timeline, queue health, and dogfood operations ## Validation - all required GitHub checks pass - full control-plane, web, bot, shared, Python, build, typecheck, lint, format, integration, and Terraform validation jobs pass - targeted D1 Autofix integration passes ## Rollout Autofix remains disabled by default. This PR does not enable any production repository. Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com> * Accept producer-agnostic Open Inspect reviews (#1183) ## Summary - accepts actionable submitted reviews authored by the exact configured Open Inspect App login and Bot actor type - keeps the dedicated Open Inspect review setting independent from third-party bot allowlists - rejects App-authored PR comments, approved reviews, empty reviews, and matching human logins without normal write permission - requires no producer-session metadata, publication receipt, special sandbox tool, or reviewer prompt change ## Why Autofix consumes authoritative GitHub reviews. Built-in review sessions and custom automations can continue publishing reviews through their existing GitHub mechanisms. Eligibility depends on the provider-read App identity and repository setting, not on which Open Inspect workflow produced the review. ## Stack - Depends on #1182 - Base branch: pr-feedback-autofix-human - Next: #1184 configuration, timeline, queue health, and dogfood operations ## Validation - repository typecheck, lint, and format check - full affected shared, control-plane, GitHub bot, and web suites - focused own-App eligibility and ingress tests - targeted D1 Autofix integration - Terraform format check ## Rollout Open Inspect review Autofix remains disabled by default. Existing review producers require no change.  ## Summary by CodeRabbit * **New Features** * Improved pull request feedback processing to recognize authoritative reviews from the configured Open Inspect app. * Actionable reviews can now be queued without an additional permission check. * Inline-only review comments are supported. * **Bug Fixes** * Improved filtering for unauthorized bots, bot comments, disabled review handling, non-actionable reviews, and reviewers without write permission. * Removed an incorrect attribution-based rejection case.  --------- Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com> * Add Autofix configuration and operations (#1184) ## Summary - adds global and repository-override Autofix settings with default-off behavior - explains that exact Open Inspect App reviews are eligible regardless of producer workflow - warns operators before trusting third-party bot input or raising attempt limits - labels admitted feedback with the existing generic review origin in the session timeline - adds primary Queue and DLQ health inspection without delaying scheduled work - documents producer-neutral dogfood, triage, and kill-switch procedures - makes warranted originating-PR outcome responses explicit ## Stack - Depends on #1183 - Base branch: pr-feedback-autofix-open-inspect-review - Final PR in the stack ## Validation - all required GitHub checks pass - full control-plane, web, bot, shared, Python, build, typecheck, lint, format, integration, and Terraform validation jobs pass - independent thermo review and closure re-review pass - independent revised-plan adherence review passes with no deviations ## Dogfood gates This PR does not enable a repository. Before dogfood: - configure external alert routing for Queue and DLQ health events - exercise both the built-in reviewer and an existing custom review automation - verify duplicate delivery, timeline provenance, and attempt-cap behavior - explicitly accept the absence of an authoritative spend budget or add that platform capability first  ## Summary by CodeRabbit * **New Features** * Added GitHub PR feedback Autofix settings, including review/comment triggers, approved bot accounts, and attempt limits. * Added per-repository Autofix overrides. * Session timelines now show whether work resumed from a human or bot comment/review, with a link to the feedback. * GitHub avatars now use stable profile images. * **Bug Fixes** * Improved Autofix queue monitoring and operational alerts. * **Documentation** * Added a rollout and troubleshooting runbook for PR Feedback Autofix.  --------- Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com> * Render rich pull request tool events in the timeline (#1618) ## Summary - replace the generic `create-pull-request` argument/output disclosure with the selected pull request preview treatment - render agent-authored PR bodies as sanitized Markdown without assuming Summary or Verification sections - parse current created, updated, draft, manual, pending, and failure output variants while preserving unknown output verbatim - validate external PR links and keep long descriptions progressively disclosed - add focused coverage for rendering, lifecycle states, unsafe URLs, arbitrary body formats, and case-insensitive tool dispatch ## Verification - `npm test -w @open-inspect/web -- src/components/create-pull-request-event.test.tsx src/components/tool-call-item.test.tsx` - `npm run lint -w @open-inspect/web` - `npm run typecheck -w @open-inspect/web` - `git diff --check` ## Testing note - the full web suite completed all 1,226 assertions successfully, but Vitest exited nonzero because the pre-existing `sandbox-settings.test.tsx` timeout callback fired after jsdom teardown (`window is not defined`) --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/6e4947f5c6a40da91e6ca16c2823cbb7)*  ## Summary by CodeRabbit * **New Features** * Added rich pull-request timeline events for creation, updates, drafts, pending states, failures, and manual creation. * Added expandable descriptions with Markdown support, branch details, links, and status indicators. * Added safe handling for external links and unrecognized pull-request output. * **Bug Fixes** * Pull-request tool calls now consistently use the specialized display, including mixed-case names. * **Tests** * Added comprehensive coverage for pull-request states, expansion behavior, link safety, and fallback rendering.  --------- Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com> * refactor(control-plane): make Autofix handler a class (#1624) ## Summary - replace the Autofix session HTTP handler factory with a class - inject `SessionAutofixService` directly through the constructor - update session composition and handler tests to use the class API - preserve the existing route adapter, validation, logging, and response behavior ## Context This aligns the Autofix endpoint with the class-based session HTTP handler pattern established in #1612. ## TDD - changed the handler test to instantiate `AutofixHandler`, confirming the red state with `AutofixHandler is not a constructor` - implemented the class and reran the focused test to green ## Validation - `npm run build -w @open-inspect/shared` - focused Autofix handler tests: 2 passed - `npm test -w @open-inspect/control-plane`: 3,253 passed - `npm run test:integration -w @open-inspect/control-plane`: 1,006 passed - `npm run typecheck -w @open-inspect/control-plane` - `npm run lint -w @open-inspect/control-plane` - targeted Prettier check - `npm run build -w @open-inspect/control-plane` - `git diff --check` --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/a531a7ca7d9557ead0ead1e406f70652)*  ## Summary by CodeRabbit * **Bug Fixes** * Maintained autofix request handling, validation, error responses, and service dispatch behavior. * Updated internal handler wiring without changing the user-visible autofix experience.  Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com> * Refactor Scheduler methods to return typed application results (#1620) ## Summary - replace `Response` return values from Scheduler tick, event, manual trigger, run completion, and health operations with operation-specific typed results - remove the synthetic `Scheduler.dispatch()` HTTP router after confirming it had no production callers - serialize Scheduler outcomes only in the real automation and webhook HTTP adapters while preserving their status codes and JSON bodies - make in-process automation completion acknowledgement and retryable failure outcomes explicit, retaining the existing two-attempt retry policy without interpreting HTTP statuses - update Scheduler unit and integration tests to invoke typed application methods directly, while retaining route/webhook HTTP contract coverage ## External Contract Preservation - manual trigger success remains `201` with `{ invocationId, runs }` - active manual runs remain `409` with `{ error: "A run is already active for this automation" }` - trigger launch failures and authoritative lookup/validation failures remain wrapped as `500` by the public route - normalized event, generic automation webhook, and Sentry webhook success bodies remain `{ ok: true, triggered, skipped, steered }` - event forwarding exceptions remain `502` at the normalized event adapter - request validation and authentication continue to run before Scheduler invocation ## Completion And Retry Behavior - completed and ignored run callbacks are explicit acknowledged outcomes - invalid callback input is an explicit retryable Scheduler failure, preserving the previous behavior where the callback service retried a non-2xx Scheduler response - thrown D1/application failures still retry once and remain distinct from typed Scheduler rejections - completion remains best-effort after both attempts, matching existing notification behavior ## Dispatch Removal Evidence Repository-wide call inspection found `Scheduler.dispatch()` only in Scheduler unit/integration test shims. Production invokes `tick()`, `event()`, `trigger()`, and `runComplete()` directly, and there is no external Scheduler service or Durable Object binding. The fake router and its unknown-route tests were therefore removed rather than retained as a compatibility layer. ## Verification - `npm test -w @open-inspect/control-plane -- src/scheduler/scheduler.test.ts src/routes/automations.test.ts src/session/callback-notification-service.test.ts src/webhooks/automation-event.test.ts src/webhooks/automation-webhook.test.ts` (229 tests) - `npm run test:integration -w @open-inspect/control-plane -- test/integration/scheduler.test.ts test/integration/scheduler-events.test.ts test/integration/scheduler-slack-events.test.ts test/integration/webhooks.test.ts test/integration/webhooks-slack.test.ts test/integration/webhooks-github-pr-lifecycle.test.ts` (85 tests) - `npm run typecheck -w @open-inspect/control-plane` - `npm run lint -w @open-inspect/control-plane` - `npm run build -w @open-inspect/control-plane` - code-simplifier review completed; no generic result framework or compatibility adapter was introduced ## Migration Impact No database, shared-package, deployment, or external API migration is required. This is an internal control-plane application boundary change; direct TypeScript callers now consume discriminated results instead of decoding synthetic HTTP responses. --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/2576829fb50115431a5a2451edc7128f)* --------- Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com> * fix: decode image build provenance at API boundary (#1626) ## Summary - replace the storage-shaped image-build status DTO with a camelCase public API contract - expose `repositoryShas` as validated `RepositoryShaEntry[] | null` instead of leaking the D1 JSON string - keep snake_case rows and `repository_shas` internal to control-plane persistence - decode each status row once at the control-plane response boundary and map malformed historical provenance to `null` - move the canonical repository provenance Zod schemas into `@open-inspect/shared` and reuse them for callback and stored-row validation - remove the web JSON parser and consume typed provenance directly while preserving status folding, fingerprint filtering, primary SHA display, and duration formatting ## HTTP Contract Image-build status records now use public camelCase names, including `scopeKind`, `scopeId`, `repositoriesFingerprint`, `runtimeVersion`, `buildDurationSeconds`, `errorMessage`, and `createdAt`. `repositoryShas` is a decoded array or `null`; `repository_shas` and all other D1 encodings are no longer exposed. Malformed historical `repository_shas` values do not fail the status feed. They map to `repositoryShas: null`. Internal rebuild and finalization paths continue reading the raw row and retain their existing invalid-provenance behavior. ## TDD Evidence ### Red Tests were changed before production code and produced the expected failures: - shared DTO tests rejected the new camelCase structured record and `repositoryShaEntrySchema` was not exported - the control-plane mapper test failed because `status-view` did not exist - status integration tests observed snake_case keys, a JSON-encoded `repository_shas`, and no nullable decoded field - web folding returned no statuses because it still read snake_case fields - primary SHA extraction returned `null` because it still expected a JSON string ### Green The minimum implementation added the shared schema, internal storage-row type, one response mapper, and typed web consumption. Focused shared, control-plane, integration, and web tests then passed. ### Refactor After green, the code-simplifier pass removed a duplicate inherited storage field and consolidated imports. The focused suites remained green. ## Compatibility All in-repo HTTP consumers are updated atomically in this monorepo. No temporary dual-field response is included: retaining `repository_shas` would continue exposing the storage encoding and conflict with the A03 contract, while there is no external consumer evidence requiring it. Shared-package changes trigger both affected deployment paths; a brief mixed-version rolling window remains the normal risk for this intentional contract change, but adding a second wire shape would not eliminate that risk without preserving the deprecated leak. ## Validation - `npm run build -w @open-inspect/shared` - shared tests: 50 files, 697 tests passed - control-plane unit tests: 213 files, 3,257 tests passed - control-plane `image-builds.test.ts` integration: 51 tests passed - web tests: 163 files, 1,231 tests passed - `npm run typecheck` - ESLint on all changed files - Prettier check on all changed files - `git diff --check` The first parallel full web run had two unrelated ESLint-boundary test timeouts under concurrent load; the isolated full web rerun passed all 1,231 tests. Repository-wide `npm run lint` and `npm run format:check` remain blocked by pre-existing, untouched `.opencode` lint errors and `.opencode/package.json` formatting drift; all changed files pass both checks. ## Migration And Risk - no D1 schema or data migration is required - malformed persisted provenance is represented safely only at the public response boundary - no image callback lifecycle or provider behavior was refactored - the intentional HTTP DTO change is the primary compatibility risk --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/529a68bb06a61cfc493c4f4414bee068)* --------- Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com> * refactor(control-plane): remove Autofix service middle-man (#1625) ## Summary - remove `SessionAutofixService`, which only forwarded two commands to `SessionMessageQueue` - give `AutofixHandler` a consumer-owned two-method queue surface - dispatch admission and recovery commands directly at the validated HTTP boundary - move both dispatch cases into the handler test and delete the duplicate service suite ## Context This addresses the second Autofix refactor finding after #1624: the session path no longer inserts a behavior-free service between the HTTP handler and message queue. ## TDD - changed the handler tests to inject queue capabilities directly and added recovery lookup coverage - confirmed the red state for both valid command variants at the old `service.handle` seam - removed the service and implemented direct narrow-port dispatch ## Validation - `npm run build -w @open-inspect/shared` - focused Autofix handler tests: 3 passed - `npm test -w @open-inspect/control-plane`: 3,252 passed - `npm run test:integration -w @open-inspect/control-plane`: 1,006 passed - `npm run typecheck -w @open-inspect/control-plane` - `npm run lint -w @open-inspect/control-plane` - targeted Prettier check - `npm run build -w @open-inspect/control-plane` - `git diff --check` --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/a531a7ca7d9557ead0ead1e406f70652)*  ## Summary by CodeRabbit * **Bug Fixes** * Autofix requests now correctly enqueue new feedback and retrieve results for recovery lookups. * Invalid autofix commands continue to return a validation error without triggering queue operations. * Autofix responses now consistently reflect whether feedback was accepted, duplicated, rejected, found, or unavailable. * **Tests** * Expanded coverage for feedback enqueueing, recovery lookups, invalid-command handling, and response outcomes.  --------- Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com> Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com> * fix: ignore injected OpenCode files in ESLint (#1622) ## Summary - exclude the session-injected `.opencode` directory from the root ESLint scan - keep generated local tooling from producing environment-specific `no-undef` and unused-variable failures ## Verification - `npm run lint` - `npx prettier --check eslint.config.js` - `git diff --check` --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/b34c42382069ae3b2941c82dc52bbe17)*  ## Summary by CodeRabbit * **Chores** * Improved linting coverage for OpenCode configuration and scripts. * Updated lint checks to recognize Node.js environments and handle intentionally unused parameters consistently.  --------- Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com> Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com> * refactor(control-plane): split sandbox event processor by family (#1629) Phase B of the collaborator-arity program: `SessionSandboxEventProcessor` was a 19-parameter dispatch table — 13+ event types, each branch using a different collaborator subset. This splits it into a thin router plus per-family handlers, mirroring the HTTP-route decomposition. Behavior-preserving: the existing `sandbox-events` suite (37 tests) passes with **zero assertion changes** — only the construction helper changed, and it now builds the real family composition. ## Shape `src/session/sandbox-events/`: | Class | Params | Owns | | --- | --- | --- | | `SessionSandboxEventProcessor` (router) | 8 | arrival logging, per-event context (one `Date.now()`, one message-attribution resolution), dispatch, **the ack contract** | | `SandboxStreamingEventHandler` | 6 | `token`, `context_compacted`, `step_start`/`step_finish`, `tool_call` + the generic timeline path (`tool_result`, `error`, `warning`, `user_message`, unknown) | | `SandboxArtifactEventHandler` | 4 | `artifact` | | `SandboxExecutionEventHandler` | 12 | `execution_complete` — the settle-a-turn convergence point | | `SandboxRuntimeEventHandler` | 7 | `heartbeat`, `session_title`, `ready`, `git_sync` | | `SandboxPushCoordinator` | 4 + resolver state | `pushBranchToRemote` and `push_complete`/`push_error` — one unit, because the terminal events settle state the request side created | The ack contract is now a single post-dispatch line in the router; family handlers never see `ackId`. Ack ordering is unchanged — critical events ack after their handler finishes, exactly where the old branches acked (`execution_complete` after `processMessageQueue`, push/tail events after broadcast). The execution handler is deliberately still wide (12): every param is a distinct role in settling a finished turn. The status-owner campaign is expected to absorb `projectTerminalMessage` and parts of `statusService` into one projection surface; the class doc says to re-measure then rather than split further now. ## Inventory findings (charted before cutting) - `error` and `snapshot_ready` had no dedicated branches — the old fall-through tail was really a *timeline-observer* path (persist → broadcast → ack-if-critical). That path is now `recordTimelineEvent` on the streaming handler, with the router's `default` case routing to it. - `ready` did its side effects early and then **fell through** to the tail (persist + broadcast). It's now fully owned by the runtime handler with the same effect order. - `snapshot_ready` in `CRITICAL_EVENT_TYPES` is unreachable: it's not in the `sandboxEventSchema` union (both entry paths validate against it) and the Modal bridge never emits it. Left inert here — flagging for a separate cleanup rather than changing semantics in a refactor. One non-observable ordering note: the router computes context (two pure reads) before dispatch, so for `ready` the `getProcessingMessage` read now precedes `pinBaselines` instead of following it; the two touch disjoint state. ## Verification - `tsc` ×3 programs (src, test, integration) clean; ESLint clean - Unit battery 3253/3253; integration battery 1006/1006 (includes `session-do-collaborator-wiring.test.ts`, which patches `pushBranchToRemote` through the DO — the router keeps that method as a delegate to the coordinator so the seam still intercepts)  ## Summary by CodeRabbit * **Improvements** * Improved processing of sandbox activity, including streaming updates, artifacts, runtime events, and execution completion. * Improved reliability of branch push operations, including completion tracking, error handling, timeouts, and support for multiple pending pushes. * Preserved delivery acknowledgements for critical sandbox events. * **Bug Fixes** * Improved session activity, status updates, notifications, and timeline synchronization during sandbox operations.  * fix: type and centralize Modal sandbox HTTP boundaries (#1631) ## Summary - add strict Pydantic request models for interactive sandbox create and snapshot restore - validate repository owner/name pairs and nested multi-repository identities at the HTTP boundary - parse create/restore requests once and construct manager inputs from typed values - centralize authentication, timing, HTTP exception tracking, generic exception mapping, and `modal.http_request` logging in a small async context manager - map unexpected internal failures to sanitized HTTP 500 responses instead of HTTP 200 `{ success: false }` payloads - preserve explicit build-session not-found handling and all endpoint-specific success response shapes ## Compatibility The existing rolling-deployment policy is preserved independently from strict field typing: - unknown top-level request fields remain ignored through `_ModalRequestModel` (`extra="ignore"`) - unknown nested restore `session_config` fields remain preserved (`extra="allow"`) so snapshots can round-trip fields introduced by newer control-plane deployments - known fields use strict types, so values such as `"false"` are rejected rather than coerced to truthy booleans - optional no-repository sessions remain supported, while partial repository identities are rejected - default timeout and VNC behavior, repo-image create behavior, snapshot clone-token compatibility, environment variables, settings, code-server/VNC/Slack flags, multi-repository session configuration, and structured correlation IDs are preserved No control-plane changes were necessary. Its Modal client already handles non-2xx responses explicitly, and successful response payloads are unchanged. ## Error Envelope The shared endpoint execution seam owns: - bearer authentication before request and control-plane URL validation - request timing and success/error outcome tracking - propagation of known `HTTPException` status/detail values - logging unexpected exceptions server-side and mapping them to bounded `500 Internal server error` responses - final `modal.http_request` logging, including endpoint-specific trace/request/session/sandbox/build identifiers Control-plane URL validation no longer reflects the submitted URL in client-visible errors. ## TDD Evidence Red: - added focused tests before production changes - initial focused run: `11 failed, 30 passed` - expected failures showed string booleans being accepted, malformed typed fields reaching Modal/domain code, and generic create/restore failures returning normally instead of raising HTTP 500 Green: - added the create/restore request models and applied the minimal execution seam to those handlers - focused create/restore run: `41 passed` Refactor: - extracted all remaining authenticated endpoint envelopes onto the tested seam - combined focused create/build API run after extraction: `74 passed` - applied the code-simplifier review and removed only redundant execution-path state and an unreachable error mapping - reran focused and full verification after refactoring ## Verification - `uv run pytest tests/test_web_api_create_sandbox.py tests/test_web_api_build_sandbox.py -q` -> 74 passed - `uv run pytest tests/ -q` -> 210 passed - `uv run ruff check src/web_api.py tests/test_web_api_create_sandbox.py` -> passed - `uv run ruff format --check src/web_api.py tests/test_web_api_create_sandbox.py` -> passed - `git diff --check` -> passed An additional `uv run mypy src/web_api.py` was attempted and reports 16 existing strict-typing issues in this legacy module, primarily pre-existing unparameterized endpoint `dict` annotations and dynamically re-exported constants. This check is not part of the requested Modal validation set and no new mypy-specific scope was added. ## Risks - malformed create/restore payloads that previously reached domain code or were silently coerced now receive HTTP 400 errors - unexpected failures now correctly produce non-2xx responses; callers relying on the erroneous HTTP-200 error object behavior will observe the corrected contract - unknown-field handling remains intentionally permissive for rolling deployments as described above ## Scope This change is limited to audit finding A21. It does not include A22's `SandboxProvider` capability/launch-contract refactor, provider adapter consolidation, or image-build lifecycle changes. --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/14275a8cddd1b305bd607af44c6f6ba0)* --------- Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com> * feat(bots): let the Slack and Linear classifiers run on OpenAI, and bound the request (#1408) ## Problem The Slack and Linear bots classify each inbound message to decide which repository or environment a coding session should target. Both classifiers are pinned to Anthropic: - `packages/slack-bot/src/classifier/index.ts` builds an Anthropic client and forces a `classify_target` tool call. - `packages/linear-bot/src/classifier/index.ts` calls `api.anthropic.com/v1/messages` directly and **hardcodes** `claude-haiku-4-5` with no env override at all. Two consequences: 1. **Single-provider coupling.** An Anthropic outage, rate limit, or billing lapse degrades routing on every deployment, with no way to point the classifier elsewhere — even for deployments whose coding agents already run OpenAI models. We hit exactly this: an Anthropic billing lapse dropped both bots to "pick a target yourself" until it was noticed. 2. **Unbounded requests.** Neither classifier passes an abort signal, so a stalled or queued provider request holds the Slack thread / Linear webhook open until the platform kills the invocation. The classifiers already fail soft to a target picker, so a *fast* failure is cheap — it was the unbounded wait that hurt. ## What this does Lets an operator pick the classifier's provider, requires **only that provider's** credential, and binds exactly one provider key to the bots. | `classification_model` | Provider | Credential required | |---|---|---| | `anthropic/<x>` or bare `claude-*` (default) | Anthropic, existing tool-calling request | `classification_anthropic_api_key`, falling back to `anthropic_api_key` | | `openai/<x>` or bare `gpt-*` | OpenAI Chat Completions, strict `json_schema` | `classification_openai_api_key` | The prefix rule reuses the convention already encoded in `normalizeModelId`/`MODEL_CATALOG` in `packages/shared/src/models.ts`, so there is no second setting that can disagree with the model id. The bare id is sent to the provider. An unrecognised prefix throws into each classifier's existing `catch`, which already degrades to asking the user to pick — no new failure mode. Both providers funnel through the existing validators (`normalizeModelResponse` in slack-bot, `classifyToolInputSchema` in linear-bot), so the downstream contract is untouched. `CLASSIFICATION_REQUEST_TIMEOUT_MS = 15_000` now bounds **both** providers, following the existing convention (`REPOS_FETCH_TIMEOUT_MS`, `OUTBOUND_REQUEST_TIMEOUT_MS`): milliseconds in the name, defined once, and asserted in tests by identity of the signal object rather than just its shape. ### Scope of the credential choice — please read This is deliberately **classifier-scoped**, not a deployment-wide provider switch. `anthropic_api_key` is left exactly as it is on `main` (`nullable = false`, non-blank validation) because it has consumers unrelated to classification: the Modal sandbox's `llm-api-keys` secret (`modal.tf`) that Claude coding sessions use, and the opencomputer control-plane path. The diff to `variables.tf` is purely additive — it does not touch that variable. So: choosing the OpenAI classifier means you supply `classification_openai_api_key` and the bots receive **only** that key. It does not make the deployment OpenAI-only, and this PR makes no claim to. Making sandbox provider credentials uniformly optional is a separate, larger change tied to the default coding model, and I have not attempted it here. ## Backward compatibility **Nothing changes for an existing deployment that sets no new value.** - `classification_model` defaults to `claude-haiku-4-5` — today's value. - `classification_anthropic_api_key` defaults to blank and falls back to `anthropic_api_key`, so existing deployments keep working untouched. - The Anthropic request body is unchanged; the timeout is passed as `messages.create(body, { signal })`, so the body itself is untouched. - `ANTHROPIC_API_KEY` stays required, the `@anthropic-ai/sdk` dependency stays, `CLASSIFY_TARGET_TOOL` stays. - No Claude entries removed anywhere — `packages/linear-bot/src/model-resolution.ts` (`MODEL_LABEL_MAP`) is untouched, so `model:opus`-style Linear labels keep working. - Anthropic-classifier deployments keep exactly the bot secret bindings they had; no empty secret is introduced and no worker version churns from this change. - The Anthropic SDK client is now constructed lazily, so an OpenAI-configured deployment never reaches `new Anthropic({ apiKey: undefined })`. The Linear bot gains a `CLASSIFICATION_MODEL` binding it never had; its default makes the previously hardcoded `claude-haiku-4-5` explicit, so the effective model is unchanged. ## Configuration ```hcl # Default — Anthropic, using the key you already supply # classification_model = "claude-haiku-4-5" # Or classify on OpenAI; the bots then receive only this key classification_model = "gpt-5.4-mini" classification_openai_api_key = "sk-proj-..." ``` Each provider's key is validated non-blank **when that provider is selected and a classifier bot is enabled** — so an OpenAI deployment is never asked for an Anthropic classifier key, a deployment running neither bot is never asked for either, and a selected provider can't ship credential-less. That last guard matters because GitHub Actions renders an unset secret as an empty string, which would otherwise plan and apply cleanly and leave a classifier rejecting every message. For the same reason the workflow maps the model with an explicit fallback (`${{ vars.CLASSIFICATION_MODEL || 'claude-haiku-4-5' }}`, matching the existing `ENABLE_SLACK_BOT || 'true'` pattern), and the configuration additionally refuses a blank override rather than silently treating it as "use the default". ## Verification Terraform (`terraform test`, mock providers) — **18 passed, 0 failed**, including a new `tests/classifier_provider.tftest.hcl` whose 8 runs cover every branch: - Anthropic default binds `ANTHROPIC_API_KEY` and **no** `OPENAI_API_KEY` on both bots (the backward-compatibility guarantee, asserted rather than assumed) - OpenAI model binds `OPENAI_API_KEY` and **no** `ANTHROPIC_API_KEY` — exactly one provider credential reaches the bots, asserted in both polarities - `gpt-5.4-mini` and `openai/gpt-5.4-mini` both resolve to OpenAI; `anthropic/claude-haiku-4-5` resolves to Anthropic - OpenAI model with a blank key → plan **fails** - OpenAI model with both bots disabled and a blank key → plan **succeeds** - unknown provider prefix → plan **fails**; blank model → plan **fails** The pre-existing `anthropic_api_key_blank` guard in `tests/auth_provider_configuration.tftest.hcl` still passes unchanged. `terraform fmt -check -recursive` clean; `terraform validate` success. TypeScript: `npm run typecheck` exit 0; `eslint --max-warnings 0` clean on both changed packages. Unit suites (clean upstream-main baseline → this branch): slack-bot 421 → **425**, linear-bot 223 → **230**; unchanged elsewhere: shared **601**, github-bot **130**, control-plane **2518**, web **956**. Control-plane integration (workerd + real D1): **778 passed**. New tests per bot cover: the OpenAI request contract (`max_completion_tokens` present, `max_tokens` absent, `temperature: 0`, `strict: true`, bare model id, `additionalProperties: false`, all fields `required`, nullable id typed `["string","null"]`), non-2xx degrading to the picker, the timeout signal being the exact `AbortSignal.timeout` object, the Anthropic default path still firing when nothing is set, and an unrecognised prefix degrading without calling either provider. ## Notes for reviewers - **`max_completion_tokens` is required and `max_tokens` is rejected** by the gpt-5 family (`Unsupported parameter: 'max_tokens' is not supported with this model`) — verified against the live API, and pinned by a test in each bot so it cannot regress silently. - Each bot implements its own small OpenAI request function rather than sharing one: two call sites with different schemas, and it keeps each Worker self-contained. Happy to extract into `packages/shared` if you would prefer that. - The provider is derived from the model id rather than a separate `CLASSIFICATION_PROVIDER` variable, to avoid a setting that can disagree with the model. If you would rather support OpenAI-compatible gateways (Azure, OpenRouter, proxies) whose ids are not `gpt-*`, an explicit provider override is the natural follow-up — happy to add it here or later. - `classification_anthropic_api_key` exists mainly so the two providers are symmetric and the classifier's credential is separable from the sandbox's. If you would rather the Anthropic classifier just always read `anthropic_api_key` and drop that variable, that is a one-line simplification — say which you prefer. - The `docs/GETTING_STARTED.md` diff looks larger than it is: adding `CLASSIFICATION_ANTHROPIC_API_KEY` widened the Actions-secret table's first column, so Prettier (which your `lint-staged` runs on Markdown) realigned every row. `git diff -w` on that file shows only the six sample lines, the two new table rows, and the widened separator.  ## Summary by CodeRabbit * **New Features** * Added configurable classification model selection for Slack and Linear bots. * Added OpenAI and Anthropic classification support with provider-specific credentials. * Added structured response validation and 15-second request timeouts. * Added graceful handling for unsupported models, provider errors, and missing credentials. * **Documentation** * Updated setup and deployment guidance for models and API keys. * **Tests** * Expanded coverage for provider selection, validation, timeouts, credentials, and fallback behavior.  * fix(types): replace unsafe casts with validated parsing (#1623) This is an automated nightly unsafe-cast remediation sweep. It fixes three current default-branch findings by replacing unsafe boundary/persisted-data assertions with Zod parsing or existing schema parsing, following the TypeScript Coding Standards unsafe-cast / parse-don't-assert guidance and the Zod boundary-validation pattern established in PR #807. | file:line | risk | cast removed | fix | | --- | --- | --- | --- | | `packages/slack-bot/src/classifier/index.ts:141` / `:152` / `:175` | High | External LLM tool payload cast to `Record<string, unknown>` and confidence cast to `ClassificationResult["confidence"]` | Added local `llmResponseSchema` and `safeParse` at the model-output boundary; invalid output preserves the existing low-confidence clarification fallback. | | `packages/control-plane/src/db/automation-model-provider-auth.ts:30` | High | Persisted provider auth rows assembled and cast to `ModelProviderSelections`, bypassing existing schema | Runs `modelProviderSelectionsSchema.parse` after row assembly so the shared Zod schema remains the source of truth. | | `packages/control-plane/src/db/mcp-servers.ts:66`, `:79`, `:94`, `:237` | Medium | Persisted MCP JSON/type fields cast to `Record<string, string>` and `"local" | "remote"` | Added package-local Zod parsers for MCP server type, command arrays, and env/header maps at D1 decode sites. | Verification: | command | result | | --- | --- | | `npm run build -w @open-inspect/shared` | Passed | | `npm run build -w @open-inspect/control-plane` | Passed | | `npm run build -w @open-inspect/slack-bot` | Passed | | `npm run typecheck` | Passed | | `npm run format` | Passed | | `npm run lint -w @open-inspect/control-plane` | Passed | | `npm run lint -w @open-inspect/slack-bot` | Passed | | `npm test -w @open-inspect/control-plane` | Passed | | `npm test -w @open-inspect/slack-bot` | Passed | | `npm run lint` | Failed on pre-existing `.opencode/**/*.js` `no-undef` errors outside this sweep's allowed touch set; package lint for changed code passed. | --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/c7e806bd601ed64888d77b3ed7ec687e)* --------- Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com> Co-authored-by: Cole Murray <colemurray.cs@gmail.com> * fix(types): validate unsafe boundary casts (#1438) This is an automated nightly unsafe-cast remediation sweep. It replaces selected unsafe TypeScript casts at boundary/persisted-data sites with parse-don't-assert validation, following the TypeScript Coding Standards unsafe-cast guidance and the Zod boundary-validation pattern established in PR #807. | Finding | Risk | Cast removed | Fix | | --- | --- | --- | --- | | `packages/slack-bot/src/classifier/repos.ts:224` | HIGH | KV fallback `cached as SlackRoutingRule[]`, bypassing the existing shared routing-rule schema | Uses `z.array(slackRoutingRuleSchema).safeParse(cached)` before `normalizeRoutingRules`; malformed cached routing rules fail open to the existing empty fallback. | | `packages/control-plane/src/session/event-stream.ts:119` | MEDIUM | persisted event `JSON.parse(event.data) as Record<string, unknown>` | Adds a local Zod `persistedEventDataSchema` and validates parsed event data before returning the HTTP event response. | | `packages/control-plane/src/routes/session-children.ts:127` | LOW | child response `(await response.clone().json()) as { messageId?: unknown }` | Replaces the assertion with a plain object/property guard; malformed best-effort response payloads continue to be ignored. | Verification: | Command | Result | | --- | --- | | `npm run format` | Passed | | `npm test -w @open-inspect/control-plane -- src/session/event-stream.test.ts src/routes/session-children.test.ts` | Passed, 2 files / 19 tests | | `npm test -w @open-inspect/slack-bot -- src/classifier/repos.test.ts` | Passed, 1 file / 23 tests | | `npm run build -w @open-inspect/shared` | Passed | | `npm run build -w @open-inspect/control-plane` | Passed | | `npm run build -w @open-inspect/slack-bot` | Passed | | `npm test -w @open-inspect/control-plane` | Passed, 168 files / 2568 tests | | `npm test -w @open-inspect/slack-bot` | Passed, 34 files / 423 tests | | `npm run typecheck` | Passed | | `npm run lint -w @open-inspect/control-plane` | Passed | | `npm run lint -w @open-inspect/slack-bot` | Passed | | `git diff --check` | Passed | | `npm run lint` | Failed on pre-existing `.opencode/` helper files (`no-undef` for `process`, `fetch`, `Headers`, `URL`, etc.), unrelated to the files touched by this sweep. | Reference: TypeScript Coding Standards unsafe-cast / parse-don't-assert guidance and the Zod webhook normalizer pattern from PR #807. --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/db6e4a50d71c0639ad6c7d522af6683f)* --------- Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com> Co-authored-by: Cole Murray <colemurray.cs@gmail.com> * fix(types): replace unsafe casts with boundary guards (#1610) This is an automated nightly unsafe-cast remediation sweep. It replaces qualifying unsafe TypeScript assertions at trust boundaries with parse-don't-assert style guards, following the TypeScript Coding Standards for unsafe casts and the Zod boundary-validation pattern established in PR #807. This PR is draft because the exact root `npm run lint` gate fails in this sandbox on untracked local `.opencode/` tooling files outside the repository-tracked source changes. | Finding | Risk | Cast Removed | Fix | | --- | --- | --- | --- | | `packages/control-plane/src/sandbox/e2b-rest-client.ts:183` | High | External E2B Connect end-stream body cast to `{ error?: { message?: string } }` | Inline `isRecord` guard before reading `error.message` | | `packages/control-plane/src/sandbox/e2b-rest-client.ts:190` | High | External E2B Connect event body cast to `{ event?: Record<string, { status?: string }> }` | Inline `isRecord` guards before reading `event.end.status` | | `packages/control-plane/src/webhooks/automation-event.ts:56` and `:83` | High | Normalized webhook envelope body cast to `Record<string, unknown>` before schema validation | Inline `isRecord` guard before source/eventType reads; existing `automationEventSchema.safeParse` remains authoritative | | `packages/web/src/app/api/sessions/[id]/title/parse-request.ts:4` | Medium | Request body cast to `{ title?: unknown }` | Existing object guard plus `"title" in body` one-field access | Verification: | Command | Result | | --- | --- | | `npm run build -w @open-inspect/shared` | Passed | | `npm run build -w @open-inspect/control-plane` | Passed | | `NODE_ENV=production npm run build -w @open-inspect/web` | Passed | | `npm run typecheck` | Passed | | `npm run format` | Passed | | `npm test -w @open-inspect/control-plane` | Passed: 204 files, 3184 tests | | `npm test -w @open-inspect/web -- src/app/api/sessions/[id]/title/route.test.ts` | Passed: 1 file, 3 tests | | `npm test -w @open-inspect/web` | Passed on retry: 162 files, 1214 tests | | `npm run lint -w @open-inspect/control-plane && npm run lint -w @open-inspect/web` | Passed | | `npm run lint` | Failed: ESLint includes untracked local `.opencode/` tooling files with `no-undef` errors; none are tracked or modified by this PR | Notes: - The first `npm run build -w @open-inspect/web` failed with this sandbox's non-standard `NODE_ENV`; rerunning with `NODE_ENV=production` passed. - No dependencies were added. --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/84e35873f963231ea86abe832d6fc1bb)* --------- Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com> Co-authored-by: Cole Murray <colemurray.cs@gmail.com> * fix(types): validate sqlite migration rows (#1593) This is an automated nightly unsafe-cast remediation sweep. It replaces three unsafe casts of opaque SQLite PRAGMA rows with a local parse/guard path, following the TypeScript Coding Standards for unsafe casts and parse-don't-assert. The selected boundary is package-local and trivial, so this uses inline runtime guards instead of Zod; this is consistent with the Zod boundary-validation pattern established in PR #807 for structured external payloads while keeping one-field SQLite row parsing minimal. | Finding | Risk | Cast removed | Fix | | --- | --- | --- | --- | | `packages/control-plane/src/session/schema.ts:386` | Medium | `PRAGMA table_info(participants).toArray() as Array<{ name: string }>` | Inline `isRecord`/`parseSqlColumnNames` guard before building the column set | | `packages/control-plane/src/session/schema.ts:422` | Medium | `PRAGMA table_info(${table}).toArray() as Array<{ name: string }>` | Inline `isRecord`/`parseSqlColumnNames` guard before checking for `scm_provider` | | `packages/control-plane/src/session/schema.ts:436` | Medium | `PRAGMA table_info(session).toArray() as Array<{ name: string }>` | Inline `isRecord`/`parseSqlColumnNames` guard before building the column set | Verification: | Command | Result | | --- | --- | | `npm run build -w @open-inspect/shared` | Passed | | `npm run build -w @open-inspect/control-plane` | Passed | | `npm run typecheck` | Passed | | `npm run format` | Passed, no additional changes | | `npm run lint -w @open-inspect/control-plane` | Passed | | `npm test -w @open-inspect/control-plane` | Passed, 203 files / 3167 tests | | `npm run lint` | Failed on pre-existing `.opencode/**` no-undef issues outside this sweep's allowed file scope | --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/c23740fe74b7a02f5cf2c5a127178219)* --------- Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com> Co-authored-by: Cole Murray <colemurray.cs@gmail.com> * fix(types): validate unsafe web boundary casts (#1563) Automated nightly unsafe-cast remediation sweep. This PR fixes two remaining web-package unsafe cast sites by parsing or narrowing boundary/opaque data instead of asserting, following the TypeScript Coding Standards unsafe-cast / parse-don't-assert guidance and the Zod boundary-validation pattern established in PR #807. | Finding | Risk | Cast removed | Fix | | --- | --- | --- | --- | | `packages/web/src/lib/tasks.ts:41` | Medium | `latestTodoWrite.args as TodoWriteArgs` for opaque sandbox tool-call args | Added a local Zod schema for the consumed TodoWrite args and `safeParse`; malformed args preserve the existing empty-list behavior. | | `packages/web/src/components/settings/data-controls-settings.tsx:72` | High | `await res.json()` trusted as `SessionListResponse` for archived-session pagination | Added a canonical session-list response schema and shared fetcher used by initial and load-more requests; malformed responses hit the existing catch/log path. | Verification: | Command | Result | | --- | --- | | `npm run build -w @open-inspect/shared` | Passed | | `npm run build -w @open-inspect/web` | Passed | | `npm run typecheck` | Passed | | `npm run format` | Passed | | `npm test -w @open-inspect/web -- --run src/lib/tasks.test.ts src/components/settings/data-controls-settings.test.tsx` | Passed | | `npm test -w @open-inspect/web` | Passed: 157 files, 1159 tests | | `npm run lint -w @open-inspect/web` | Passed | | `npm run lint` | Failed on pre-existing `.opencode/` JavaScript globals (`Headers`, `fetch`, `process`, etc.) outside the touched files; PR opened as draft per sweep instructions. | --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/7c0bca8b4624321b48bb19ce9a137ee6)* --------- Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com> Co-authored-by: Codex <codex@openai.com> Co-authored-by: Cole Murray <colemurray.cs@gmail.com> * fix(types): replace unsafe persisted-data casts (#1520) This is an automated nightly unsafe-cast remediation sweep. It replaces selected unsafe TypeScript assertions over persisted or loose boundary data with runtime narrowing, preserving existing null/skip behavior for malformed values and leaving valid inputs unchanged. The fixes follow the TypeScript Coding Standards unsafe-cast / parse-don't-assert guidance and the Zod boundary-validation pattern established in PR #807; these particular findings were simple persisted-data shapes, so lightweight inline guards were sufficient and no dependency changes were made. | Finding | Risk | Cast removed | Fix | | --- | --- | --- | --- | | `packages/control-plane/src/session/tunnel-urls.ts:28` | Medium | `parsed as Record<string, string>` after parsing stored `sandbox.tunnel_urls` JSON | Inline guard builds a fresh `Record<string, string>` only after validating every entry | | `packages/control-plane/src/session/pr-artifacts.ts:20` | Medium | `parsed as { repoOwner?: unknown; repoName?: unknown }` after parsing stored PR artifact metadata | Inline `isRecord` guard before reading repo identity fields; malformed metadata still returns `null` | | `packages/control-plane/src/sandbox/lifecycle/image-selection.ts:125` | Medium | `primary as { baseSha?: unknown }` after parsing stored `repository_shas` JSON | Inline `isRecord` guard before reading `baseSha`; malformed provenance still yields `null` | | `packages/web/src/lib/session-socket/artifact-metadata.ts:65` | Medium | `artifact.metadata as Record<string, unknown> | null` from loose session artifact wire metadata | Inline `isRecord` guard before UI metadata narrowing; non-object metadata is ignored | Verification: | Command | Result | | --- | --- | | `npm run build -w @open-inspect/shared` | Passed | | `npm run build -w @open-inspect/control-plane` | Passed | | `NODE_ENV=production npm run build -w @open-inspect/web` | Passed | | `npm run typecheck` | Passed | | `npm run lint -- --ignore-pattern '.opencode/**'` | Passed; `.opencode` is untracked local tooling in this workspace and is excluded from the PR | | `npm run lint -w @open-inspect/control-plane` | Passed | | `npm run lint -w @open-inspect/web` | Passed | | `npm test -w @open-inspect/control-plane` | Passed | | `npm test -w @open-inspect/web` | Passed when run isolated; concurrent run with control-plane tests timed out in two existing ESLint-boundary tests, then passed on isolated rerun | | `npm run format` | Passed | | `git diff --check` | Passed | --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/9c22735b7a63f6e49a3d58042e10a5bd)* --------- Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com> Co-authored-by: Cole Murray <colemurray.cs@gmail.com> * fix(types): validate unsafe boundary casts (#1462) | Finding | Risk | Cast removed | Fix | | --- | --- | --- | --- | | `packages/control-plane/src/routes/session-ws-token.ts:23` | HIGH | `parseJsonBody<{ scmLogin?: string; scmName?: string; scmEmail?: string }>` generic request-body assertion for an auth/session token path | Added a local Zod schema and `safeParse` after preserving raw-body identity enforcement | | `packages/control-plane/src/routes/image-builds.ts:339` | HIGH | `parseJsonBody<{ enabled?: unknown }>` generic request-body assertion feeding repo image-build persistence | Parsed JSON as `unknown` and used an inline record/boolean guard before persistence | | `packages/control-plane/src/routes/session-child-spawn.ts:97` | MEDIUM | `(await spawnContextRes.json()) as { error?: unknown }` on an opaque session-runtime response | Parsed as `unknown` and used an inline record/string guard, preserving the existing fallback message | Verification: | Command | Result | | --- | --- | | `npm run format` | Passed | | `npm test -w @open-inspect/control-plane` | Passed, 172 files / 2598 tests | | `npm run build -w @open-inspect/shared` | Passed | | `npm run build -w @open-inspect/control-plane` | Passed | | `npm run typecheck` | Passed | | `npm run lint -w @open-inspect/control-plane` | Passed | | `git diff --check` | Passed | | `npm run lint` | Failed on pre-existing `.opencode` files (`process`, `fetch`, `Headers`, etc. reported as undefined), unrelated to this PR | No dependency changes. --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/6347ee0b9042691211c410eacb804bcd)* --------- Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com> Co-authored-by: Cole Murray <colemurray.cs@gmail.com> * fix(types): validate unsafe boundary casts (#1419) This is an automated nightly unsafe-cast remediation sweep. It replaces selected high-risk unsafe TypeScript casts with parse-don't-assert validation at trust boundaries, following the TypeScript Coding Standards for unsafe casts and the Zod boundary-validation pattern established in PR #807. | Finding | Risk | Cast removed | Fix | | --- | --- | --- | --- | | `packages/slack-bot/src/callbacks.ts:353` | HIGH | `payload as AutomationSkipPayload` after `request.json()` | Added a local Zod `automationSkipSchema` and uses `safeParse` before signature validation and async handling. | | `packages/control-plane/src/scheduler/durable-object.ts:864` | HIGH | `event as SlackAutomationEvent` after `automationEventSchema.safeParse` | Replaced the cast with discriminant narrowing from the already-validated automation event union. | Verification: | Command | Result | | --- | --- | | `npm test -w @open-inspect/slack-bot` | Passed: 34 files, 422 tests. | | `npm test -w @open-inspect/control-plane` | Passed: 161 files, 2540 tests. | | `npm run build -w @open-inspect/shared` | Passed. | | `npm run build -w @open-inspect/control-plane` | Passed. | | `npm run build -w @open-inspect/slack-bot` | Passed. | | `npm run format` | Passed. | | `npm run typecheck` | Passed. | | `npm run lint -w @open-inspect/control-plane` | Passed. | | `npm run lint -w @open-inspect/slack-bot` | Passed. | | `npm run lint -- --ignore-pattern .opencode/` | Passed for the tracked repository tree. | | `git diff --check` | Passed. | Reference: TypeScript Coding Standards unsafe-cast / parse-don't-assert guidance and the Zod webhook normalizer pattern from PR #807. --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/13cffa9a6e1265b60e4deb0ebffcb302)* Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com> Co-authored-by: Cole Murray <colemurray.cs@gmail.com> * fix(types): validate unsafe boundary casts (#1531) This is an automated nightly unsafe-cast remediation sweep. It replaces selected high-risk TypeScript assertions at trust/persistence boundaries with parse-don't-assert validation, following the TypeScript Coding Standards for unsafe casts and the Zod boundary-validation pattern established in PR #807. | Finding | Risk | Cast removed | Fix | | --- | --- | --- | --- | | `packages/control-plane/src/routes/secrets.ts:48` | HIGH | `parseJsonBody<{ secrets?: Record<string, string> }>(request)` on an external request body before writing repo secrets | Added `secretsRequestBodySchema` and `safeParse`; invalid bodies continue returning 400 | | `packages/control-plane/src/routes/environment-secrets.ts:237` | HIGH | `parseJsonBody<{ repoOwner?: string; repoName?: string; keys?: unknown }>(request)` plus `body.keys as string[] | undefined` before authorization/import | Added `environmentSecretsImportBodySchema` and `safeParse`; invalid bodies continue returning 400 | Deduplication: the persisted event-stream finding was removed because PR #1438 already owns that remediation. Verification: | Command | Result | | --- | --- | | `npm run build -w @open-inspect/shared` | Passed | | `npm run build -w @open-inspect/control-plane` | Passed | | `npm run typecheck` | Passed | | `npm run lint` | Passed after temporarily moving the ignored, untracked local `.opencode/` runtime directory out of the workspace to match a clean checkout, then restoring it | | `npm run format` | Passed | | `npm test -w @open-inspect/control-plane` | Passed | | `npm test -w @open-inspect/control-plane -- secret-request-schemas` | Passed | --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/c0ead4082436a7d15ef260cc7e5080bb)* --------- Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com> Co-authored-by: Cole Murray <colemurray.cs@gmail.com> * fix(types): replace unsafe casts with validated parsing at boundaries (#1544) This is an automated nightly unsafe-cast remediation sweep. It replaces two selected trust-boundary assertions with parse-or-guard checks while preserving the existing throw/null behavior for invalid inputs, following the TypeScript Coding Standards guidance for unsafe casts and parse-don't-assert boundaries. The Zod callback validation follows the boundary-validation pattern established in PR #807. | Finding | Risk | Cast removed | Fix | | --- | --- | --- | --- | | `packages/web/src/hooks/use-provider-accounts.ts:61` | HIGH | `(await response.json().catch(...)) as { error?: string; retryable?: boolean }` for API error bodies | Replaced with inline `Record<string, unknown>` guard for consumed `error` and `retryable` fields; malformed bodies keep falling back to the existing default error. | | `packages/web/src/hooks/use-warm-draft-session.ts:96` | HIGH | `(await response.json()) as { sessionId?: unknown }` for create-session response | Replaced with inline `Record<string, unknown>` guard and string/non-empty `sessionId` check; malformed responses keep returning `null`. | Deduplication: the Slack automation-skip callback finding was removed because PR #1419 already owns that remediation. Verification: | Command | Result | | --- | --- | | `npm run build -w @open-inspect/shared` | Passed | | `NODE_ENV=production npm run build -w @open-inspect/web` | Passed. The same build without overriding `NODE_ENV` failed in this environment while prerendering `/_global-error` after Next warned about a non-standard `NODE_ENV`. | | `npm run typecheck` | Passed | | `npm run lint -w @open-inspect/web` | Passed | | `npm run format` | Passed | | `npm test -w @open-inspect/web -- use-provider-accounts use-warm-draft-session` | Passed | | `npm test -w @open-inspect/web` | Passed on rerun: 156 files, 1145 tests. Initial full run had two ESLint-boundary test timeouts that passed when rerun directly. | --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/ce3d5cc7d25737250789e60698702458)* --------- Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com> Co-authored-by: Cole Murray <colemurray.cs@gmail.com> * fix(types): validate boundary casts (#1502) Automated unsafe-cast remediation pass for user-preference and session-creation boundaries. - Defines the shared `userPreferencesSchema` as the canonical source for `UserPreferences`. - Validates Linear and Slack KV payloads with the shared schema before returning preferences. - Validates warm-draft session creation responses with `createSessionResponseSchema`, including malformed JSON handling. - Adds focused shared, Linear, Slack, and web regression coverage. The branch was reconciled with the latest `main`; the warm-draft overlap preserves the canonical response schema and fail-closed malformed-JSON behavior. --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/bb4d2859edb33b81cd0b5847b8444014)* Co-authored-by: waclaude <colemurray.cs+ghwa…

fix: type Modal sandbox HTTP requests

3526061