chore: sync fork with upstream (ColeMurray/background-agents) - #19
Merged
Conversation
while working on ColeMurray#1037 i noticed that the e2b sandboxes started by the current template were failing to run bun despite being installed by the dockerfile. The Dockerfile previously ran the installer like this: `BUN_INSTALL=/usr/local curl ... | bash` That environment variable applied to `curl`, not the `bash` process running the installer. Bun therefore used its default install location, which was outside the runtime user's PATH. This change passes `BUN_INSTALL=/usr/local` to `bash` and also adds `command -v bun` to the template readiness check. ### Before <img width="1228" height="755" alt="e2b-bun-issue-before" src="https://github.com/user-attachments/assets/781533c4-5983-4262-bcf8-acb0cdddcf26" /> ### After <img width="1231" height="782" alt="e2b-bun-issue-after" src="https://github.com/user-attachments/assets/7258ce2b-8f53-42f3-9a98-2a8603181fa5" /> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Template readiness checks now verify that Bun is available before finalization. * **Chores** * Improved the Bun installation setup during environment creation. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…asses (ColeMurray#1612) ## Summary Item 3 of the deps-style normalization campaign (follow-up to ColeMurray#1608/ColeMurray#1609): the seven session HTTP handlers still built as `createXHandler(deps)` factories over deps-bags become classes with direct constructor collaborators, matching the `SessionDiffsHandler` (ColeMurray#1047) and `AttachmentsHandler` precedents. One prerequisite commit makes `TOKEN_ENCRYPTION_KEY` required, mirroring ColeMurray#1609's treatment of the repo-secrets key. The deps-bags were where most of the composition root's pure same-name forwards lived — closures like `getSession: () => sessionCoreRepository.getSession()` that exist only because a bag can't hold the repository itself. Net effect in `components.ts`: 43 function-valued closure lines removed, 8 added back as named per-request adapters (−35), and all seven `XHandlerDeps` interfaces deleted. ## `TOKEN_ENCRYPTION_KEY` is now required (first commit) Terraform already requires the key (no default, `sensitive`) and the `Env` type declares it non-optional — the three falsy-guards were silent-degradation branches: - `identity.ts` silently dropped stored SCM tokens from GitHub enrichment, - the session graph silently skipped constructing the user token store, - session init silently discarded a plaintext SCM token instead of encrypting it. `requireTokenEncryptionKey(env)` shares the AES-256 material validator with `requireRepoSecretsEncryptionKey` (strict base64, exactly 32 decoded bytes) and is thrown at session-graph construction, so a misconfigured deployment fails every request at init rather than degrading. Plaintext-read paths are untouched. ## Conversion rules (uniform across all seven) - **Collaborators become constructor params with their real types** — repositories, services, messenger. `deps.getSession()` → `this.sessionCoreRepository.getSession()`. - **Constant thunks become data** — `getDurableObjectId: () => durableObjectId` → `durableObjectId: string`; `isManagedSecretsConfigured: () => Boolean(db)` → `managedSecretsConfigured: boolean` (fixed at composition). - **Module functions re-wrapped only to bind composition-time values are called directly** — `resolvePublicSessionId(session, this.durableObjectId)`, `parseArtifactMetadata(artifact, this.log)`, `validateReasoningEffort(model, effort, this.log)`; same instances, same arguments as the deleted closures. - **Genuine adapters stay function-typed params** (8 total): the three per-request token/credential service factories on `SandboxHandler`, the request-log-scoped `createPullRequest` factory + `getSessionUrl` + background `triggerPullRequestRefresh` on `PullRequestHandler`, and `scheduleWarmSandbox` + `cancelSession` on `SessionLifecycleHandler`. - **Seams stay functions without eta-expansion** — the root passes `generateId`/`hashToken`/`encryptToken`/`isValidSandboxToken` as bare module references; `now` defaults to `Date.now` per the `AttachmentsHandler` precedent. - **The class replaces the same-named interface**, so the internal route table (`components.ts` tier 9) is untouched — those wrappers adapt the uniform route signature to method arities and are not forwards. - `SessionLifecycleHandler`'s cancel path reuses the lifecycle `WebSocketManager` port via a `LifecycleSocketAdapter` instance (ColeMurray#1608) instead of two raw socket forwards; the adapter's `sendToSandbox` performs the identical resolve-then-send. - `PullRequestHandler`'s local result-union aliases were byte-identical to `ParticipantService`'s declared return types and are deleted. ## Behavior notes - Behavior-preserving except the deliberate key-requirement change above. - Tests now exercise the real `resolvePublicSessionId` (via `session_name` fixtures) and the real `validateReasoningEffort` (whose catalog answers match what the old stubs returned) instead of stubs. - One commit per handler group; every commit is independently green. ## Testing - `tsc --noEmit` (prod + test configs), ESLint, Prettier - Unit: 205 files / 3186 tests green - Integration (workerd + real D1): green <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added validation for the token encryption key used to protect OAuth tokens. * Token-based identity enrichment now requires valid encryption-key configuration. * **Bug Fixes** * Improved configuration errors for missing, malformed, or incorrectly sized encryption keys. * **Refactor** * Updated session and HTTP request handling for more consistent dependency management without changing endpoint behavior. * **Tests** * Expanded coverage for encryption-key validation and token-related session flows. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Behavior-preserving follow-up to ColeMurray#1608/ColeMurray#1609/ColeMurray#1612 (deps-style normalization, per the ColeMurray#1045–ColeMurray#1049 standard): drop the vestigial logger thunks. Five sites took the session logger as a zero-arg function (`getLogger: () => Logger` / `getLog: () => Logger`) and called it on every use; all five are fed a value that is constant after composition, so they now take `log: Logger` directly. The thunks existed for the DO-era log swap: `SessionDO` used to reassign its logger once the public session id resolved, so anything that captured a logger by value at construction time kept logging the stale id. That mechanism is gone — the composition root builds one session-scoped logger whose `session_id` is injected **per emit** through the latched resolver (`components.ts`: "for every component in the graph, however early it captured the logger"). The comment in `sandbox-events.ts` justifying its getter ("The DO swaps its logger for a request-scoped child during fetch()") described behavior that no longer exists. ## Changes | Site | Before | After | | --- | --- | --- | | `SessionHttpDispatcher` deps | `getLogger: () => Logger` | `log: Logger` | | `SessionMessageRouter` deps | `getLogger: () => Logger` | `log: Logger` | | `SessionDisconnectHandler` deps | `getLogger: () => Logger` | `log: Logger` | | `SessionSandboxEventProcessor` ctor | `getLog: () => Logger` + `private get log()` accessor | `private readonly log: Logger` (accessor deleted; internal `this.log` uses unchanged) | | `createCloudflareBackgroundTasks` | `getLogger: () => Logger = () => log` | `logger: Logger = log` (worker/scheduler callers use the default, unchanged) | Composition root: the three `getLogger: () => log` props and two `() => log` arguments become `log`. ## What deliberately stays a function Everything that is genuinely dynamic, per the campaign's classification: - **Latched resolvers** — `getSessionId` (DO id until the session row exists, public id after). - **Live queries** — `getStatus`, `getAuthenticatedClients`, `getSandboxSocket`, `getProcessingMessageAuthor`, `isSpawning`. - **Post-init freshness reads** — `getExecutionTimeoutMs`. - **The SCM provider cell** — `() => scmProvider` reads a mutable `let` that live-DO integration tests substitute after graph construction. - **Clock/id seams and adapters** — `now`, `generateId`, action-shaped deps. ## Testing - `npm run typecheck -w @open-inspect/control-plane` (both tsconfigs) clean - `npm run lint -w @open-inspect/control-plane` clean - Unit: 3187 passed; integration: 1002 passed <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Updated session and background task components to receive logging instances directly. * Streamlined error, request, message, disconnect, and sandbox-event logging. * Preserved existing session handling, cleanup, reconnection, and close behavior. * **Tests** * Updated automated tests and test setup to match the simplified logging configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary `test/integration/**` (91 files) was never typechecked — eslint covers `src/` only, and the tsconfigs excluded the directory. Store-signature drift there has repeatedly survived until runtime (`D1_TYPE_ERROR` mid-suite; most recently a stale `SandboxRepository` construction found during ColeMurray#1609). This PR adds `tsconfig.integration.json`, fixes everything it surfaced (1,033 errors initially, most from one root cause), and wires it into `npm run typecheck` so CI enforces it from now on. ## The config - Extends the production tsconfig with `types: ["@cloudflare/workers-types", "@cloudflare/vitest-pool-workers/types"]` — the integration files execute inside workerd, so they compile against workers types **without Node globals** (same boundary rationale as the prod config; Node-context files like `vitest.integration.config.ts` run in the Vite host and are not part of this program). - The pool's `cloudflare:test` declarations live at the package's `./types` subpath export (v0.16 layout). The old root-package reference silently loads nothing — which is why the existing `env.d.ts` was augmenting a `ProvidedEnv` interface that no longer exists. - `env.d.ts` rewritten to the v0.16 contract: merge the worker's real `Env` (plus `TEST_MIGRATIONS`) into the `Cloudflare.Env` placeholder that `env` from `cloudflare:test` is typed as. This one fix collapsed ~900 of the initial errors. - An experiment narrowing `SESSION` to `DurableObjectNamespace<SessionDO>` inside the augmentation was reverted: it makes `Cloudflare.Env` unassignable to the production `Env` at every `handleRequest(env)` call site. The production `Env` cannot be narrowed either — importing the DO class from `types.ts` is exactly what the only-`index.ts`-imports-the-adapter lint exists to prevent. Instead, stub typing happens at one seam: ## New test seams (all in existing helper files) | Helper | Why | | --- | --- | | `runInSessionDO(stub, cb)` | `runInDurableObject` with the stub typed as the session DO — the single cast asserting what the SESSION namespace hosts (43 call sites converted) | | `ctxOf(instance)` | the DO's `ctx` is `protected` on the `DurableObject` base class; storage seeding/assertions go through this one cast | | `sqlDatabase(env.DB)` | plain assignment (no cast) viewing D1 through the engine-neutral `SqlDatabase` interface, so tests can `batch()` store-bound statements (21 sites) | | `getSetCookies(headers)` | workerd implements `Headers.getSetCookie()` but this workers-types version doesn't declare it — same cast `src/routes/browser-auth.ts` carries | ## Latent drift the checker caught (the point of the exercise) All fixed behavior-preservingly: - **`AutomationRow` fixtures still carried `repo_owner`/`repo_name`/`base_branch`/`repo_id`** (6 files) — dead since repos moved to the `automation_repositories` junction table; linkage in the affected tests already flows through `replaceRepositories(...)`. - **Run fixtures set `concurrency_key`** — it lives on invocations now, so the seeded value never reached any table. Note for a follow-up: the scheduler-events "does not block a different concurrency key" test seeds its active run without any key either way, so it doesn't currently distinguish per-key scoping from no-key blocking (left as-is; runtime unchanged). - **Browser-auth router tests passed a raw `ExecutionContext` where the router now takes `BackgroundTasks`** (3 files) — worked only because the failure path never ran. Now wrapped with `createCloudflareBackgroundTasks`, mirroring `index.ts`. - **`stubSourceControlProvider` was missing `resolveCommit`/`listTree`/`readBlob`** — the provider read-surface added for skills import; stubbed with the suite's existing `notUsedHere` idiom. - **A session fixture wrote status `"initializing"`** — removed from the status vocabulary (ColeMurray#1554); now `"active"`. - **`generateId({ model: "user" })`** — Better Auth's canonical generator takes no arguments; the argument was silently ignored. - **`ensureInitialized` still passed in a `SessionPlatform` stub** — unthreaded by ColeMurray#1604. - **Repository skill assignments missing the now-required `baseBranch`**, and **image-build correlation contexts missing the required `trace_id`**. Plus mechanical strictness fixes (WebCrypto union narrowing in the Google id-token helper, `json<T>()` typing, non-null assertions where `subscribe: true` guarantees replay messages). `session-do-access.ts`'s old comment — "test/integration/** is never typechecked (eslint + grep are the only static gates here)" — is retired. ## Testing - `npm run typecheck` (now three programs) clean - Unit: 3187 passed; integration: 1002 passed — no behavioral change - Prettier over the touched files <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Improved integration-test coverage and type-checking across authentication, sessions, automations, scheduling, webhooks, and Durable Object workflows. * Updated test infrastructure for more reliable cookie handling, database batching, background tasks, and session state access. * Refined fixtures and assertions to reflect current repository, concurrency, and session behavior. * **Chores** * Updated test TypeScript configurations and runtime type definitions for improved validation and editor support. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary - count bridge heartbeats as sandbox activity while a message is processing - keep idle heartbeats liveness-only so abandoned sandboxes still reach inactivity cleanup - add unit and Durable Object integration coverage for both states ## Motivation A long-running tool call can emit no agent events for longer than the sandbox inactivity timeout even though the bridge remains healthy. Previously, bridge heartbeats refreshed only heartbeat liveness, so the lifecycle alarm could classify the sandbox as idle and stop it mid-execution. The sandbox event processor already owns which incoming events count as activity. While a message is processing, a live bridge heartbeat now renews the existing activity timestamp. After processing finishes, heartbeats no longer renew activity and ordinary idle cleanup remains unchanged. This is a deliberately narrow alternative to ColeMurray#1601. It does not change execution-timeout recovery, provider stop behavior, queue recovery, schema, or cleanup semantics. ## Validation - npm test -w @open-inspect/control-plane — 205 files, 3,188 tests passed - npm run test:integration -w @open-inspect/control-plane — 81 files, 1,002 tests passed - npm run typecheck -w @open-inspect/control-plane - npm run lint --workspace=@open-inspect/control-plane -- --no-fix - Prettier check for all changed files - git diff --check origin/main...HEAD <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved heartbeat tracking so idle heartbeats maintain liveness without incorrectly extending activity timers. * Heartbeats received while processing a message now correctly refresh activity status. * Heartbeat events continue to be excluded from stored event history. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Closes out the deps-style normalization campaign (ColeMurray#1608/ColeMurray#1609/ColeMurray#1612/ColeMurray#1615/ColeMurray#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 ColeMurray#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 (ColeMurray#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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## 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. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## 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)* <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## 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. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com>
## 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. ColeMurray#1183: producer-agnostic Open Inspect App reviews 3. ColeMurray#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>
## 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 ColeMurray#1182 - Base branch: pr-feedback-autofix-human - Next: ColeMurray#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. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## 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. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com>
## 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 ColeMurray#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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## 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. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com>
## 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)* <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## 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. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com>
## 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 ColeMurray#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)* <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## 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. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com>
…urray#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>
## 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>
…y#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 ColeMurray#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)* <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## 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. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com> Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com>
## 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)* <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## 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. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com> Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com>
…eMurray#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) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## 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. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## 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>
…ound the request (ColeMurray#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. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## 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. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
) 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 ColeMurray#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>
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 ColeMurray#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 ColeMurray#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>
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 ColeMurray#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>
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 ColeMurray#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>
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 ColeMurray#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>
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 ColeMurray#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>
| 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>
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 ColeMurray#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 ColeMurray#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>
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 ColeMurray#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 ColeMurray#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>
…ColeMurray#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 ColeMurray#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 ColeMurray#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>
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+ghwaclaude@gmail.com> Co-authored-by: Cole Murray <colemurray.cs@gmail.com>
This automated unsafe-cast remediation sweep validates three TypeScript boundaries, following the repository's parse-don't-assert standard and the shared-schema validation pattern established in PR ColeMurray#807. | Boundary | Change | Failure behavior | | --- | --- | --- | | Persisted Autofix origin in `session/message-queue.ts` | Parse with the existing shared `githubAutofixOriginSchema` instead of asserting JSON as `GitHubAutofixOrigin` | Log `prompt.invalid_origin_context` and omit malformed origin metadata while continuing prompt dispatch | | Opaque Autofix activity cursor in `db/pr-autofix-feedback-store.ts` | Guard the decoded JSON as a non-array record before applying the existing finite timestamp and nonempty feedback-key checks | Preserve `Invalid Autofix activity cursor` and reject before querying | | Invocation history rows in `db/automation-store.ts` | Parse SQL rows using a package-local Zod schema with the shared invocation-status schema; validate the count result too | Reject storage-integrity errors explicitly instead of silently dropping rows or replacing malformed counts with zero; preserve valid nullable fields | The review follow-up merges current `main`, preserves its identity/lifecycle changes, and corrects the original invocation parser's silent row omission so an invalid page cannot be returned as successful but incomplete history. It also simplifies origin parsing within the existing catch boundary. Regression coverage includes all three valid Autofix origin variants; invalid origin JSON/shapes/URLs; malformed cursor encodings, primitive/array/null values, non-finite timestamps, and invalid fields; and malformed invocation fields. Existing real D1 tests validate the complete derived-status truth table, nullable timestamps, mixed invocation history, and Autofix cursor pagination. Verification after the review follow-up: | Command | Result | | --- | --- | | `npm run build -w @open-inspect/shared` | Passed | | `npm test -w @open-inspect/control-plane -- src/db/automation-store-invocations.test.ts src/db/pr-autofix-feedback-store.test.ts src/session/message-queue.test.ts` | Passed: 3 files, 111 tests | | `npm test -w @open-inspect/control-plane -- src/session/message-queue.test.ts` | Passed after the final origin-parser simplification: 92 tests | | `npm run test:integration -w @open-inspect/control-plane -- test/integration/automation-invocations.test.ts test/integration/pr-autofix-feedback-store.test.ts` | Passed: 2 files, 24 real Workerd/D1 tests | | Prettier check of all six changed files | Passed | All checks on the final reviewed head `359381d90c86ce85fc7c7fe45a5a71e645234bfb` pass: TypeScript lint/format and typecheck, web build, control-plane unit tests and both integration shards, web and bot tests, Compose container smoke, and Terraform validation. Terraform Plan/Apply are intentionally skipped for this PR workflow. --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/3f5322363922279dfe02ee7ad87f2757)* --------- Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com> Co-authored-by: Cole Murray <colemurray.cs@gmail.com>
This is an automated nightly unsafe-cast remediation sweep. It replaces three selected unsafe assertions of persisted session data with package-local Zod row schemas and `safeParse` at the session SQLite read boundaries, following the TypeScript Coding Standards guidance for unsafe-cast / parse-don't-assert and the Zod boundary-validation pattern established in PR ColeMurray#807. | Finding | Risk | Cast removed | Final behavior | | --- | --- | --- | --- | | `packages/control-plane/src/session/artifact-repository.ts` | MEDIUM | `result.toArray() as ArtifactRow[]` | `artifactRowSchema` validates artifact reads. Missing single rows return `null`; malformed existing rows throw `SessionStorageIntegrityError`, preserving duplicate-PR protection and authoritative artifact discovery. Nullable `url` and `metadata` remain valid. | | `packages/control-plane/src/session/participant-repository.ts` | MEDIUM | `result.toArray() as ParticipantRow[]` in identity, ID, list, and WebSocket token reads | `participantRowSchema` validates participant rows. Identity/ID/list reads throw `SessionStorageIntegrityError` for malformed rows instead of treating corruption as permission to create a duplicate participant. The token-auth lookup intentionally returns `null` for missing or invalid rows to fail closed. Nullable identity/token columns remain valid. | | `packages/control-plane/src/session/alarm/scheduler.ts` | MEDIUM | `.toArray() as AlarmStateRow[]` | A package-local schema validates the alarm singleton, including `cancelled` constrained to `0 | 1`. A missing row remains absent; malformed persisted state throws `SessionStorageIntegrityError` so cancellation and pending/in-flight deadlines cannot silently disappear. Nullable deadlines remain valid. | The follow-up integrity-error change addresses all three original review findings. The branch has also been updated with current main, preserving its alarm scheduling and Node SQLite adapter changes. Original implementation 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 | | `npm run format` | Passed | | `npm test -w @open-inspect/control-plane` | Passed, 230 files / 3454 tests | | `git diff --check` | Passed | Current-head verification is recorded in this PR's checks. Regression coverage includes malformed artifact/participant rows, nullable persisted fields, malformed alarm state, and invalid cancellation values. References: TypeScript Coding Standards unsafe-cast / parse-don't-assert guidance, and the Zod boundary-validation pattern established in PR ColeMurray#807. --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/f086839106eb58a4d674a121dd1e666a)* --------- Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com> Co-authored-by: Cole Murray <colemurray.cs@gmail.com>
This is an automated nightly unsafe-cast remediation sweep. It fixes two uncovered unsafe TypeScript casts by validating/parsing boundary-shaped data instead of asserting it, while excluding findings already addressed by open `automation:unsafe-cast` PRs. This follows the TypeScript Coding Standards guidance for unsafe-cast / parse-don't-assert and the Zod boundary-validation pattern established in PR ColeMurray#807; these two selected fixes are lightweight inline guards because the consumed shapes are local and small. | Finding | Risk | Cast removed | Fix | | --- | --- | --- | --- | | `scripts/bootstrap-workspace-owner.ts:232` | High | `JSON.parse(stdout) as WranglerResult[]` for external Wrangler CLI JSON controlling Owner bootstrap reporting | Added `parseWranglerResults` with runtime array/object/result-row guards before consuming rows | | `packages/shared/src/triggers/slack/conditions.ts:56` | Medium | `value as { pattern?: unknown; flags?: unknown }` for untrusted persisted Slack `text_match` condition values | Added a direct record guard and read `pattern` / `flags` from the guarded value | Verification: | Command | Result | | --- | --- | | `npm run format` | Passed | | `npm run build -w @open-inspect/shared` | Passed | | `npm run typecheck` | Passed | | `npm run lint` | Passed | | `npm test -w @open-inspect/shared` | Passed, 53 files / 807 tests | | `npm run test:rbac-bootstrap-owner` | Passed, 14 tests | No dependency changes were made. Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com>
This is an automated nightly unsafe-cast remediation sweep. It replaces selected unsafe TypeScript casts at external/opaque JSON boundaries with local parse-and-validate guards, following the TypeScript Coding Standards guidance for unsafe casts / parse-don't-assert and the Zod boundary-validation pattern established in PR ColeMurray#807. These two script-local boundaries do not justify a new dependency, so the fixes use narrow inline structural guards instead of adding a schema library at the root. | Finding | Risk | Cast removed | Fix | | --- | --- | --- | --- | | `scripts/cf-logs.ts` | Low | Cloudflare telemetry `await response.json()` asserted as a response object | Validate the success flag, optional response envelopes, and the timestamp/string fields actually consumed by formatting and summaries. Reject malformed events instead of silently dropping them; preserve optional empty event results and retain unconsumed fields in `--json` output. | | `scripts/merge-split-users.ts` | Medium | Wrangler D1 `JSON.parse(child.stdout)` asserted as `WranglerQueryResult[]` | Validate query envelopes, record rows, and optional nonnegative integer change counts. Reject malformed rows instead of turning them into an empty result. Preserve statement-failure diagnostics and enforce one result per submitted statement, including single-query verification reads. | The branch incorporates current main's user-merge CLI runner/test seam and atomic result-bearing batch behavior. No new dependency is introduced. ## Verification | Check | Result | | --- | --- | | `npm run test:user-merge-cli` | Passed: 18 tests, including malformed query results, failure diagnostics, empty successful results, and positional batch results. | | `npm run test:cf-logs-cli` | Passed: 20 tests exercising the real CLI with mocked fetch, including valid formatted/raw output, optional empty results, malformed events/envelopes, and API errors. | | Changed-file ESLint and Prettier | Passed. | | Strict TypeScript check for Cloudflare logs script/tests | Passed. | | `git diff --check` | Passed. | | Latest-head GitHub CI | Passed for `3d4429a9c8668209f1a4230f30bec584de06ea59`: lint/format (including both CLI test suites), workspace typecheck, web build, control-plane unit tests, both integration shards, web tests, bot tests, Compose smoke, and Terraform validation. Terraform Plan/Apply are expected skips. | Both CLI suites run in the TypeScript CI workflow. Tests do not access live Cloudflare services or production databases. An additional direct strict typecheck of the user-merge script reproduces main's existing `SqlStatement` to `{ render(): string }` assertion diagnostic; the JSON-boundary changes introduce no additional diagnostics in that check. --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/832c6a646bea50576716e060ac13b03a)* Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com> Co-authored-by: Cole Murray <colemurray.cs@gmail.com>
This is an automated nightly unsafe-cast remediation sweep. It replaces three uncovered unsafe TypeScript casts at boundary responses with Zod parsing instead of asserting, while excluding findings already covered by open `automation:unsafe-cast` PRs. The changes follow the TypeScript Coding Standards guidance for unsafe-cast / parse-don't-assert and the Zod boundary-validation pattern established in PR ColeMurray#807. | Finding | Risk | Cast removed | Fix | | --- | --- | --- | --- | | `packages/web/src/app/api/repos/route.ts:32` | High | `await response.json()` asserted as `ControlPlaneReposResponse`, bypassing the existing shared `controlPlaneReposResponseSchema` | Reused the shared Zod schema with inline `safeParse` before returning repositories | | `packages/control-plane/src/webhooks/github.ts:58` | High | Session DO artifact response asserted as `{ artifacts?: SessionArtifactSummary[] }`, feeding PR lifecycle session-state mirroring | Added package-local `sessionArtifactSummarySchema` / `sessionArtifactsResponseSchema` and `safeParse`; invalid or malformed artifact responses fail closed to the existing empty-artifact path | | `packages/web/src/app/api/integrations/slack/channels/route.ts:29` | Medium | `await response.json()` asserted as `ControlPlaneChannelsResponse` for the Slack channel picker proxy | Added shared `slackChannelListingSchema` / `controlPlaneSlackChannelsResponseSchema` and `safeParse` at the proxy boundary | | 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/web` | Failed under this shell's non-standard `NODE_ENV` with the known Next `/_global-error` prerender `useContext` error | | `NODE_ENV=production npm run build -w @open-inspect/web` | Passed | | `npm run typecheck` | Passed | | `npm run lint` | Passed | | `npm run format` | Passed | | `npm test -w @open-inspect/shared` | Passed, 53 files / 808 tests | | `npm test -w @open-inspect/control-plane` | Failed twice: unrelated `src/node/host.test.ts` timing assertion `expected true to be false`; 271 files and 3942 tests passed | | `npm test -w @open-inspect/control-plane -- src/webhooks/pull-request-lifecycle.test.ts` | Passed, 1 file / 22 tests | | `npm test -w @open-inspect/control-plane -- src/node/host.test.ts` | Failed with the same unrelated timing assertion | | `npm test -w @open-inspect/control-plane -- --maxWorkers=1` | Failed with the same unrelated timing assertion; 271 files and 3942 tests passed | | `npm test -w @open-inspect/web` | Failed once on existing ESLint-boundary test timeouts, then passed on retry with 190 files / 1461 tests | | `git diff --check` | Passed | Opened as draft because the required control-plane package test gate is currently failing on an unrelated pre-existing Node-host timing test, even though the changed webhook test and all build/typecheck/lint/format gates pass. --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/498824efe76c5f253072707596f4483d)* --------- Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com>
Automated nightly unsafe-cast remediation for TypeScript boundary safety. This sweep fixes persisted/opaque SQL result-row casts by following the unsafe-cast / parse-don't-assert standard and the Zod boundary-validation pattern established in PR ColeMurray#807. | Finding | Risk | Cast removed | Fix | | --- | --- | --- | --- | | `packages/control-plane/src/db/analytics-store.ts:93` | Medium | `(result.results ?? [])[0] as SummaryRow � undefined` | Added a package-local Zod summary row schema and `safeParse` validation before decoding. | | `packages/control-plane/src/db/analytics-store.ts:142` | Medium | `(result.results ?? []) as TimeseriesRow[]` | Added a package-local Zod timeseries row schema and `safeParse` validation for each row. | | `packages/control-plane/src/db/analytics-store.ts:215` | Medium | `((result.results ?? []) as BreakdownRow[])` | Added a package-local Zod breakdown row schema and `safeParse` validation for each row, preserving nullable `key` and `display_name`. | Verification: | Command | Result | | --- | --- | | `npm test -w @open-inspect/control-plane -- analytics-store.test.ts` | Passed: 1 file, 6 tests. | | `npm run format` | Passed; only intended files changed. | | `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. | | `npm test -w @open-inspect/control-plane` | Passed: 276 files, 4015 tests. | No dependency changes were made. --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/92ff3e7cf904fd85276f45ffdb42dacf)* Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com>
This is an automated nightly unsafe-cast remediation sweep. It addresses three currently qualifying unsafe TypeScript cast findings by parsing or guarding at the boundary, following the TypeScript Coding Standards for unsafe casts / parse-don't-assert and the Zod boundary-validation pattern established in PR ColeMurray#807. | Finding | Risk | Cast removed | Fix | | --- | --- | --- | --- | | `packages/shared/src/completion/extractor.ts:332` / `:358` / `:363` / `:411` | MEDIUM | Event payload `args` / `metadata` values were treated as records after assertion | Replaced with a plain record guard before reading consumed fields; malformed event metadata now falls back to existing display defaults | | `packages/web/src/app/api/sessions/[id]/sandbox-access/route.ts:21` | MEDIUM | Control-plane `409` JSON body was asserted as `{ error?: unknown }` | Replaced with an inline guard that only suppresses the known sandbox-unavailable conflict and preserves malformed/other conflicts | | `packages/control-plane/src/session/ws-client-mapping-repository.ts:48` | MEDIUM | D1 websocket mapping rows were asserted as `WsClientMappingResult[]` | Added a package-local Zod row schema and made `WsClientMappingResult` a `z.infer` type; nullable D1 profile fields are modeled as nullable | 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` | Passed | | `npm run format` | Passed | | `npm test -w @open-inspect/shared` | Passed: 51 files, 782 tests | | `npm test -w @open-inspect/control-plane` | Passed: 219 files, 3344 tests | | `npm test -w @open-inspect/web` | Passed on isolated final run: 170 files, 1311 tests | Note: `npm run build -w @open-inspect/web` failed once under the shell's non-standard `NODE_ENV` with a Next `/_global-error` prerender `useContext` error, then passed with `NODE_ENV=production`. Two web ESLint-boundary tests also timed out when the full web suite ran concurrently with repo-wide checks, then the isolated final web test run passed. References: TypeScript Coding Standards unsafe-cast / parse-don't-assert guidance, and the Zod boundary-validation pattern established in PR ColeMurray#807. --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/8dfceac09bfb66c6b4400bbd84fc5054)* --------- Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com> Co-authored-by: Cole Murray <colemurray.cs@gmail.com>
This is an automated nightly unsafe-cast remediation sweep. It replaces the generic `request.json()` assertion in the shared control-plane route helper with untrusted `unknown` parsing, then validates the concrete integration-settings and model-preferences request-body shapes at the boundary. This follows the TypeScript Coding Standards for unsafe-cast / parse-don't-assert and the Zod boundary-validation pattern established in PR ColeMurray#807; these shapes are one-field/trivial wrappers, so inline guards are used instead of introducing schemas. | Finding | Risk | Cast removed | Fix | | --- | --- | --- | --- | | `packages/control-plane/src/routes/shared.ts:436` | HIGH | `(await request.json()) as T` in the shared route JSON helper | Return `unknown` from `parseJsonBody`; callers must validate before field access | | `packages/control-plane/src/routes/integration-settings.ts:116`, `:226`, `:321` | HIGH | Concrete `parseJsonBody<{ settings?: Record<string, unknown> }>` uses of untrusted request JSON | Inline `isRecord`/`extractSettings` guard before global, repo, and environment settings persistence | | `packages/control-plane/src/routes/model-preferences.ts:78` | HIGH | Concrete `parseJsonBody<{ enabledModels?: unknown[] }>` use of untrusted request JSON | Inline `isRecord` guard before accepting `enabledModels` | Verification: | Command | Result | | --- | --- | | `npm run format` | Passed | | `npm run lint` | Passed | | `npm run build -w @open-inspect/shared` | Passed | | `npm run build -w @open-inspect/control-plane` | Passed | | `npm run typecheck` | Passed | | `npm test -w @open-inspect/control-plane` | Passed, 225 files / 3413 tests | | `npm run test:integration -w @open-inspect/control-plane -- integration-settings.test.ts model-preferences.test.ts` | Passed, 2 files / 48 tests | | `npm run test:integration -w @open-inspect/control-plane` | Failed outside changed coverage: unrelated workerd pool 5s timeouts in `managed-skills.test.ts`, `rbac-routes.test.ts`, `spawn-children.test.ts`, and `websocket-client.test.ts` | | `npm run test:integration -w @open-inspect/control-plane -- --maxWorkers=1` | Timed out after 600s with repeated workerd `broken.outputGateBroken` force-eviction exceptions | Opened as draft because the full integration gate did not complete cleanly in this sandbox, even though the focused integration tests for the changed routes pass. --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/aac7548f84492c7ccbeffdf4e41f1a4e)* --------- Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com> Co-authored-by: Cole Murray <colemurray.cs@gmail.com>
This is an automated nightly unsafe-cast remediation sweep. It replaces three qualifying persisted-data casts with parse-don't-assert validation at session state boundaries, following the TypeScript Coding Standards guidance for unsafe casts and the Zod boundary-validation pattern established in PR ColeMurray#807. | Finding | Risk | Cast Removed | Fix | | --- | --- | --- | --- | | `packages/control-plane/src/session/terminal-message-projection-store.ts:58` | Medium | `toArray() as PendingRow[]` for persisted terminal projection state | Added package-local Zod `pendingRowSchema` plus inline `safeParse` parser before mapping to domain state | | `packages/control-plane/src/session/message-repository.ts:106` | Medium | `result.one() as { count: number }` for unfinished prompt count | Replaced with `readRequiredNumberColumn` inline guard before queue admission uses the value | | `packages/control-plane/src/session/message-repository.ts:201` | Medium | `.one() as { count: number }` for Autofix attempt-limit count | Replaced with `readRequiredNumberColumn` inline guard before attempt-limit admission uses the value | Verification: | Command | Result | | --- | --- | | `npm run format` | Passed | | `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 | | `npm test -w @open-inspect/control-plane` | Passed, 242 files / 3591 tests | No dependency changes were made. --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/aecd1105f35bcd9be84a4eeb15a571af)* --------- Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com> Co-authored-by: Cole Murray <colemurray.cs@gmail.com>
This is an automated nightly unsafe-cast remediation sweep, updated after review to preserve existing webhook configuration and editor behavior. ## Changes - Make JSON reads through `CacheStore`, its Cloudflare KV adapter, and `SqlCacheStore` return `unknown` instead of a caller-selected generic type. - Validate cached GitHub installation tokens and repository lists with local Zod schemas; malformed entries become cache misses and are refreshed normally. - Replace webhook comparison assertions with explicit `Number`/`String` coercion while preserving the existing scalar filter schema and JavaScript comparison semantics. Existing numeric-string comparisons and numeric `contains` values remain accepted, including configurations emitted by the current editor. - Keep the webhook editor unchanged. The initial schema tightening would have rejected existing saved configurations and forced incomplete numeric edits to zero. - Strengthen cache regression fixtures to exercise validation itself: the malformed repository cache has a matching SCM identity and fresh TTL, and the malformed token has valid timestamps but an invalid token type. ## Verification - Shared TypeScript build passed. - Shared trigger-schema and webhook-normalizer tests: 34 passed, including persisted scalar compatibility and invalid non-scalar configuration cases. - Control-plane authentication, repository route, SQL cache, and Node cache conformance tests: 45 passed. - Real workerd Cloudflare KV/D1 cache conformance: 15 passed, 1 intentionally skipped because the KV clock cannot be advanced. - ESLint and Prettier checks passed for all changed files. - Full workspace `npm run typecheck` passed, including control-plane unit/integration/Node configurations, all bots, infrastructure packages, shared, and web. - All applicable GitHub checks passed at `076a30cfb8970f4ec4f79a5e1f9c53c8d1a58d24`, including both integration shards and Compose smoke; Terraform Plan/Apply are intentionally skipped for PR validation. - Merged current upstream `main` without conflicts; no dependency changes are introduced by this PR. --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/046eb6be14a088b15438070c5a9b0d51)* --------- Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com> Co-authored-by: Cole Murray <colemurray.cs@gmail.com>
<!-- react-doctor-owner: nightly-cleanup --> <!-- react-doctor-bucket: web-safe-local --> <!-- react-doctor-base-sha: d0f72a3 --> <!-- react-doctor-manifest: settings-shell:no-initialize-state,no-effect-chain --> ## Summary - Resolve the real media-query snapshot before the first client render, while exposing an unknown viewport during SSR and hydration. - Let the settings shell use that readiness directly instead of a separate mount effect or synthetic hydration store. Preserve the existing boolean media-query API for other consumers. - Remove the warning-row key changes: the rows are stateless, and presentation-derived keys introduce collisions without a demonstrated UI regression to fix. Warning identity changes are not part of this PR. ## Review feedback addressed - Fresh mobile client navigation no longer mounts the desktop settings tree before switching to mobile. The regression test uses the real media-query hook, not a resolved-value hook mock. - The content-derived warning keys and speculative reorder test have been removed completely. ## Validation - Built `@open-inspect/shared` and passed web typecheck. - Scoped ESLint and Prettier checks passed; `git diff --check` passed. - 25 tests passed across the settings shell, media-query hook, sidebar layout, session sidebar, and model selector. - Real `renderToString` / `hydrateRoot` test preserves the server busy placeholder, renders only the mobile child tree after hydration, and reports no recoverable hydration errors. - Query switching, viewport-change subscriptions, listener cleanup, and the existing boolean SSR fallback are covered. - Controlled comparison with current main: the fresh-client test records `mount, update` (the busy placeholder adds a commit). With this change it records only `mount`. - Controlled comparison with the original PR head: the same test records desktop then mobile; the corrected implementation records mobile only. Both baseline checks failed as expected, and all eight final lifecycle tests passed after restoring the corrected sources. The prior React Doctor diagnostic counts are not claimed for this narrowed revision. CI must pass on the final head before merge. --------- Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com> Co-authored-by: Cole Murray <colemurray.cs@gmail.com>
## Summary - Remove `packages/control-plane/src/realtime/events.ts`, whose `EventCategory`, `getEventCategory`, and `TokenAggregator` exports have no consumers. - Remove the colocated tests, which were the only imports of the module. - Keep the active realtime event pipeline unchanged: sandbox events continue through `SessionSandboxEventProcessor`, the family handlers, `SessionEventStream`, and `SessionWebSocketManager`. This removes 101 production lines and 179 test lines with no runtime replacement. ## Due diligence - Repository-wide code search found no consumers beyond the deleted test. - Repository docs describe the active session event pipeline and do not reference these utilities. - Git history shows the module dates to the initial implementation; subsequent cleanup removed its barrel and sibling helpers but left these self-tested exports. - Linear search included active and archived issues plus workspace documents. Exact searches for `TokenAggregator`, `getEventCategory`, `realtime/events.ts`, token aggregation/batching, event categories, and WebSocket message overhead found no planned use. ## Validation - `npm run build -w @open-inspect/shared` - `npm run typecheck -w @open-inspect/control-plane` - `npm test -w @open-inspect/control-plane -- --run` — 281 files, 4,101 tests passed - `npx eslint packages/control-plane/src --max-warnings=0` - `git diff --check` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Removals** * Removed realtime event categorization and token aggregation capabilities. * Removed buffering and flushing behavior for token events, including interval-, size-, and message-based flushing. * Removed associated test coverage. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
) ## Summary Port the two Modal deployment fixes validated in production after the shared sandbox-image consolidation (ColeMurray#1816): - Restore `/tmp` to mode `1777` before Debian/Ubuntu APT operations. The staged Modal image can leave it owned by root with mode `0755`, preventing APT's unprivileged `_apt` user from creating temporary files and causing misleading repository-signature errors. - Set `copy=True` when adding `sandbox_runtime` to the Modal function image. This bakes the source into the image so the subsequent `.env(...)` build step is valid; Modal rejects build steps after a runtime-only `add_local_dir` mount. - Include the regression guards from production for both image contracts. No provider selection, deployment configuration, credentials, or unrelated production changes are included. ## Validation - Full sandbox-images test suite. - Full Modal infrastructure test suite. - Ruff lint and format checks on changed Python files. - Bash syntax check for the Debian installer and `git diff --check`. These same runtime changes were previously verified through a successful production Modal image build, fresh-sandbox smoke verification, and function deployment. This public port does not trigger a new manual production deployment. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Runtime files are now copied into built images, ensuring builds use a fresh bundled copy. * Debian and Ubuntu sandbox images now restore `/tmp` with shared, secure permissions before package installation. * **Tests** * Added coverage verifying runtime copying and `/tmp` setup occur in the correct build order. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary - Remove the unused `requireScopedPermission` constructor and `scoped-permission` member of the internal route authorization requirement union. - Remove its unreachable enforcement branch, service-ceiling mapping, and matching test conditional. - Preserve the shared scoped-permission definitions and resolver used by live automation ownership checks. ## Why this is safe Repository searches found no callers of the constructor and no route declarations constructing this requirement directly. Documentation and Linear review found no use case tied to this unused helper. No route declarations, permission definitions, endpoints, or automation ownership behavior change. The change is intentionally limited to three files; it does not remove the active `automation` requirement or any other debt candidate. ## Validation - Shared build and control-plane Worker/Node builds - Control-plane typecheck (production, Node, unit-test, and integration-test configurations) - Full control-plane unit suite: 4,114 tests passed across 281 files - Real Worker/D1 integration: 62 tests passed across route catalog conformance, route admission matrix, automation authorization, service authentication, and audit events routes - Existing 172-route catalog and admission snapshots unchanged - ESLint and Prettier checks for the three changed files; `git diff --check` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Breaking Changes** - Removed support for scoped-permission authorization requirements in route configuration. - Routes using scoped-permission requirements no longer receive scoped-permission enforcement and should be updated to use supported permission or automation requirements. - Updated authorization validation to focus on supported permission-based requirements. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary - replace the 913-line `AutomationForm` implementation with a small orchestration component and cohesive fields for targets, triggers, model settings, and instructions - centralize form initialization, validation, and payload construction in a pure tested policy - share target-count, schedule, timezone, condition, and Slack channel validation across the web form and control plane - preserve existing create/edit payload behavior, target hydration, branch selection, event-type handling, and model coercion ## Why `AutomationForm` had accumulated orthogonal create/edit, trigger, targeting, scheduling, model, and submission branches in one component. Its ESLint cyclomatic complexity was 94, validation was duplicated between the submit handler and disabled state, and its UI state type did not accurately model the transport payload. After this refactor, `AutomationForm` is 181 lines with complexity 9. All production functions introduced or changed by the decomposition are at or below complexity 20. ## Test strategy The pure form policy was developed in red-green slices covering: - scheduled and event payload construction - repository and environment target constraints - event-type requirements and stale event types - Slack channel scope and empty channel values - Sentry create-only secrets - minimum schedule cadence Two delegated thermo-nuclear reviews were completed. The first found a UI/API validation mismatch; the second confirmed it was closed and found one inline Slack feedback inconsistency, which is also fixed and regression-tested. ## Validation - `npm run lint` - `npm run typecheck` - shared: 531 tests passed - control-plane: 2,197 tests passed - web: 865 tests passed - shared, control-plane, and web production builds passed <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Improved automation form with dedicated controls for AI model, reasoning effort, instructions, triggers, schedules, and targets. - Added repository and environment search with single- and multi-target selection. - Added live instruction character counts and trigger-specific guidance. - Added schedule timezone selection and validation. - **Bug Fixes** - Automation schedules now require a minimum 15-minute interval. - Improved validation for Slack channel conditions, including whitespace, unsupported operators, and normalized channel IDs. - Improved handling of existing GitHub filters during automation edits. - Added clearer validation for targets, event types, secrets, and required fields. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary - add vertical separation between session metadata and the budget/spend-limit section in the right sidebar ## Verification - `npm test -w @open-inspect/web -- session-right-sidebar.test.tsx` - `npm run lint -w @open-inspect/web -- --quiet` ## Visual verification The local app was captured at 1512x982, but authentication prevented reaching a populated session sidebar. The component change is covered by the targeted sidebar test. Artifact: `287c3d647a098b0a32404001b286c47b` --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/500a799f4e892af979ab623125ed18d3)* <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Removed unnecessary empty space in the session sidebar when no budget information is available. * Budget-related content now appears with consistent spacing when applicable. * **Style** * Improved the sidebar layout for sessions without cost limits or budget data. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com> Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com>
## Summary - Remove the unused `SessionIndexStore.updateTitle` method, its two self-tests, and its now-unused SQL handling in the unit-test fake. - Keep `updateTitleIfNewer`, including its current-write and stale-write regression tests. ## Why The old method unconditionally updated the D1 session title using `Date.now()`. Repository-wide reference checks found only its declaration and two self-tests; there are no production or fixture callers. Manual renames and generated titles both flow through `SessionTitleService`, which synchronizes D1 through `updateTitleIfNewer` with the authoritative update timestamp. The same-named lifecycle HTTP handler is a different method and remains unchanged, as do the title endpoints, event handling, broadcasts, and ownership/authorization checks. Documentation and Linear review found no planned use for this store method. The rename-related issue found in Linear concerns the live lifecycle handler, not this unused persistence helper. This is a focused subset of the dead-store-method audit finding: two files, 37 deleted lines, no replacement abstraction, and no database migration. Other store helpers now used by integration tests are intentionally out of scope. ## Validation - Shared build - Control-plane typecheck (production, Node, unit-test, and integration-test configurations) - 145 unit tests across session index, title service, lifecycle handler, sandbox event processor, and runtime proxy - 69 real Worker/D1 integration tests across session index, session lifecycle, and sandbox events - ESLint, Prettier, and `git diff --check` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Session title updates now reject stale writes when a newer update already exists. - Unconditional title updates are no longer supported, helping prevent newer session titles from being overwritten by outdated changes. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…ay#1812) ## Summary - Extract a local `PushOperation` owning push-spec parsing, validation, checkout selection, Git subprocess execution, timeout escalation, redaction, and error-to-result conversion. - Keep command dispatch and timestamped `push_complete` / `push_error` Session Event Stream emission in the bridge. - Preserve provider-generated URLs/refspecs/force, manifest-based repository selection, identity-free fallback, existing redaction, and branch/repository event metadata. - Move execution tests out of the bridge suite and retain boundary-mocked transport contract tests. Add coverage for nested GitLab owners, force/refspec passthrough, kill escalation, malformed specs/stderr, and launch failures. ## Verification - Focused push and bridge tests: 192 passed. - Full sandbox-runtime suite with ambient credential environment cleared: 820 passed, 3 skipped, 1 unrelated failure. - Remaining failure: `test_git_invokes_custom_ssh_signer_with_literal_public_key` expects Git to pass `-U` to the SSH signer; installed Git 2.39.5 does not. This test exercises untouched signing code. - Ruff lint and formatting checks passed for all changed files. - Commit hooks and `git diff --check` passed. Addresses issue 7, Separate Git push execution from WebSocket transport. --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/ee6a9a84a0c39f3f517282a948b03485)* <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved Git push handling with clearer success and error reporting, including branch and repository details. * Added validation for incomplete or invalid push requests. * Improved checkout selection, repository-specific workspace support, and identity-free fallback behavior. * Improved timeout and cancellation cleanup for push operations. * Added safer redaction of sensitive Git error details and clearer handling of unexpected failures. * **Tests** * Expanded coverage for push success, failures, validation, workspace selection, timeouts, cancellation, and process cleanup. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com> Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com>
## Summary - update the sandbox image agent-browser pin from 0.21.2 to 0.37.0 - refresh the generated npm tool lockfile - update the verified Linux x64 binary checksum and version fixture ## Validation - `python3 packages/sandbox-images/cli.py lock --check` - `uv run --project packages/sandbox-images --extra dev pytest packages/sandbox-images/tests -v` (55 passed) - `uv run --project packages/sandbox-images --extra dev ruff check packages/sandbox-images` - verified the downloaded `agent-browser-linux-x64` SHA-256 and `agent-browser 0.37.0` version output - `git diff --check` --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/b80cff4528400195331375d382819d78)* <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Chores** - Updated the bundled agent-browser tool to version 0.37.0. - Updated verification data to validate the newer tool release. - Installed agent-browser as a checked native binary at `/usr/local/bin/agent-browser`. - Removed the previous installation path through the package’s `node_modules` directory. - Removed agent-browser from the sandbox’s package dependency and lock metadata. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com> Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com>
…y#1814) ## Summary - Introduce an internal sandbox draft module with a typed field registry owning display, field-specific parsing and validation, inheritance, clearing, explicit resource nulls, payload construction, and dirty detection. - Replace eleven independent editor state variables with one draft and one reset, leaving the existing global/repository/environment scope adapter and UI unchanged. - Add module-interface coverage for untouched inherited values, stored overrides, resource provider defaults, cleared optional numbers, timeout conversions, dirty normalization, and effective port conflicts. - Compare parsed payloads in two component tests rather than relying on JSON property order. ## Validation - `npm test -w @open-inspect/web -- --maxWorkers=4`: 191 files, 1,510 tests passed. - `npm run typecheck -w @open-inspect/web`: passed. - ESLint for all four changed files: passed. - Prettier and `git diff --check`: passed. The initial unrestricted test run hit two 5-second timeouts in unrelated authentication-boundary ESLint tests. The complete suite passed with four workers; no timeout configuration was changed. This is an internal refactor with no intended UI or inheritance behavior changes. The existing dirty-detection behavior that ignores invalid tunnel rows when normalized valid ports are unchanged is preserved and explicitly tested. --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/efa7495e46f885cf0e9533d99c8112ab)* <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Improvements** * Sandbox settings now handle inherited values, explicit clears, saved defaults, and existing overrides more consistently. * Improved validation for ports, tunnels, resource values, session timeouts, and service-to-service conflicts. * Settings changes are normalized more reliably, including whitespace and tunnel entries. * Validation detects invalid or reserved ports and provides ordered error feedback. * Save can surface validation errors for invalid input instead of remaining disabled. * Child-session limits are validated to ensure concurrent limits do not exceed total limits. * Added validation and support for positive session cost limits. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com> Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com>
…#1828) ## Summary Reduce redundant global npm installation on the sandbox's MCP startup path. - Reuse exact pinned versions already installed globally, checking package metadata and executable links. This covers packages baked by existing setup/prebuild hooks and retained in filesystem snapshots. - Install only cache misses. Reuse successful floating-version installs during OpenCode process restarts, but refresh unversioned packages and tags on each new sandbox boot. - Persist an incomplete-install marker before invoking npm; failed, timed-out, or cancelled installs are retried. Publish the marker atomically and terminate/reap the owned npm process group on interruption. - Keep commands, server arguments, and credentials unchanged. Recognize common npx package options without interpreting server arguments as npm flags, and preserve ordering for conflicting versions of the same package. - Add cache hit/miss and preparation-duration logs. Keep the policy in a dedicated runtime module. - Advance runtime/rebuild generation to 63 while retaining compatibility floor 62, so existing snapshots remain valid. ## Scope and rollout This uses the existing prebuild lifecycle, not a new automatic MCP image-building pipeline. To eliminate the install from first-session startup, pin matching versions in the MCP command and `.openinspect/setup.sh`, then rebuild the image. Documentation includes the pattern. Floating versions retain their cross-boot refresh behavior. This removes redundant global installation; npx can still resolve registry metadata or populate its own execution cache, especially with explicit `--package` commands. No production latency reduction is claimed without measurement, and no deployment is included. ## Validation - Sandbox runtime suite: 816 passed, 3 skipped. - Final focused MCP tests: 56 passed, including a real offline npm tarball installation under a temporary prefix and reuse by newly constructed installers without another install. - Supervisor/MCP regression subset after cache bookkeeping cleanup: 63 passed. - Modal infrastructure suite: 219 passed. - Control-plane runtime manifest, prebuilt image selection, and rebuild-policy tests: 14 passed. - Shared TypeScript build, Ruff lint/format, Prettier checks, and mypy for the new cache module passed. - Regression coverage includes mixed hits/misses, pinned version changes, damaged manifests/binaries/links, floating-version refresh, conflicting package versions, failed installs across supervisor recreation, timeout/cancellation cleanup, and npm-root lookup fallback. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - MCP server packages now load faster across sandbox restarts by reusing verified installations when available. - Failed or interrupted package installations are retried automatically. - Unversioned packages are refreshed as needed to ensure current versions are used. - Invalid or incomplete package installations are detected and repaired. - Installation and cache activity now provides clearer metrics and status reporting. - **Documentation** - Added guidance covering package preinstallation, image setup, snapshots, supported options, remote servers, and command or credential forwarding. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…#1841) ## Summary Clear `last_heartbeat` atomically when reserving a replacement sandbox generation. A new sandbox has not sent a heartbeat yet and must not inherit the predecessor's liveness timestamp. ## Validated root cause Correlated Modal and control-plane logs for production session `f5674775558d2ab1aedf5e614939ea66` show this sequence on 2026-09-08 (UTC): - 04:10:04.501: snapshot restore begins. - 04:10:11.079: the replacement is still `spawning`. - 04:10:11.753: an alarm reports `sandbox.heartbeat_stale`, age 3,914,472 ms, threshold 90,000 ms; the row becomes `stale`. - 04:10:15.243: Modal restore succeeds. - 04:10:17.059: sandbox token verification returns 410 because the sandbox is `stale`; route admission translates this into a 401 for `/sandbox-skills`. - The restored supervisor exits with `ManagedSkillsError` before connecting its bridge. The heartbeat belongs to the previous sandbox, not the seven-second-old restore. `updateSandboxForSpawn()` retained it, unlike `updateSandboxForResume()`, which already clears it. Shared session alarms can fire during provider startup. Startup finalization correctly refuses to overwrite a terminal lifecycle transition, so the incorrect `stale` classification persists. The runtime's subsequent `/sandbox-error` response of 200 acknowledges and ignores a report from a stale sandbox; it does not recover the session. ## History This is a longstanding defect, not a recent regression. It is present in the root public commit `31b6df7e` (Initial open source release, 2026-01-24): restore reused the sandbox row without clearing `last_heartbeat`, and the alarm evaluated that field for `spawning` rows. Later lifecycle and repository extractions preserved the behavior. Managed skills, introduced in `97f6aeb8`, made this occurrence fail immediately at the first authenticated HTTP request. It did not introduce the invalid stale transition. ## Fix choice - Reset the generation-owned heartbeat alongside the new identity and credentials, before any provider work. - Keep heartbeat timeouts, connecting watchdogs, dead-sandbox authentication rejection, and conditional generation/status transitions unchanged. - Do not add retries, relax authentication, or overwrite genuinely stopped/superseded attempts on late provider completion. - Leave the broader restore-success logging semantics unchanged in this focused repair. ## Regression validation - Added spawn/restore interleaving tests: seed a stopped sandbox with an expired heartbeat, run an alarm inside the provider call, and require the replacement to remain spawning and finish connecting without shutdown or snapshot side effects. - Extended the shared storage conformance test to verify that an actual stored heartbeat is cleared. This runs on Node SQLite (memory and file) and real Durable Object SQLite. - Before the fix: both lifecycle cases became stale, and both Node storage variants retained the old heartbeat. - After the fix: existing heartbeat-expiry, connecting-timeout, authentication, and generation-fencing cases remain green. Checks: - Shared build passed. - 317 focused control-plane tests passed across lifecycle manager/decisions, sandbox repository, sandbox handler, and Node storage conformance. - 15 Workerd storage conformance tests passed. - Control-plane typecheck passed. - ESLint, Prettier, and `git diff --check` passed. No production state changes or deployment were performed. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Corrected sandbox startup so replaced or restored sandboxes no longer retain stale heartbeat information from a previous instance. - Improved startup state handling during replacement, keeping the sandbox in the appropriate connecting state without unnecessary websocket detachment or snapshot creation. - Added coverage to verify heartbeat reset behavior across spawn and restore scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Users can currently toggle available models and leave settings without pressing Save, unintentionally losing their changes. Save each change immediately instead. - Autosave individual model switches and category Enable all / Disable all actions. - Remove the Save button and explain that changes save automatically. - Show the updated selection immediately and a Saving... status while persisting; temporarily disable controls to prevent overlapping full-list writes. - Restore the previous selection and show an error when saving fails, allowing retry. - Update the active SWR cache after successful saves so other model selectors reflect the change. - Preserve the requirement that at least one model remains enabled, and never save initial/default state just from rendering. ## Verification - All 1,476 web tests pass across 190 files: `npm test -w @open-inspect/web -- --maxWorkers=4`. - New component coverage includes individual/category autosave, shared-cache updates, removed-model filtering, last-model protection, pending-write protection, and server/network failures with retry. - Shared build, web typecheck, ESLint on changed files, formatting, and `git diff --check` pass. - The default-concurrency full-suite run hit two authentication-lint test timeouts; the four-worker run passed without test or timeout configuration changes. - Browser visual verification was not performed: the workspace lacks local web authentication/backend configuration, and settings requires authenticated access. --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/5fb0a64c707818b0d24d6290ee14ccc9)* --------- Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com> Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com>
…ked literal (ColeMurray#1820) ## What this changes Each bot worker receives a `DEFAULT_MODEL` plain-text binding, and today the value is a literal inside tracked Terraform: - `workers-github.tf`: `"anthropic/claude-haiku-4-5"` - `workers-slack.tf`: `"claude-haiku-4-5"` - `workers-linear.tf`: `"claude-sonnet-4-6"` So a deployment that wants its bots on a different model has no configuration path. It has to edit those three tracked `.tf` files and keep carrying the edit — every `git pull` is a potential conflict, and the choice is invisible to anyone reading `terraform.tfvars`. Every other deployment-shaped decision in this environment (`classification_model`, `app_name`, `sandbox_provider`, `enable_durable_object_bindings`) is already a variable. This adds one variable per bot and binds it: - `github_bot_default_model`, default `anthropic/claude-haiku-4-5` - `slack_bot_default_model`, default `claude-haiku-4-5` - `linear_bot_default_model`, default `claude-sonnet-4-6` Each default is exactly the literal the binding already carried, so no existing deployment changes behaviour and nobody has to set anything. The three defaults deliberately stay different from one another — this is a refactor of where the value lives, not a decision to unify the bots' models. One variable per bot rather than one shared variable, because the bots consume `DEFAULT_MODEL` at different decision points and a deployment can reasonably want them to differ: the GitHub bot falls back to it when a repository's integration config pins no model (`packages/github-bot/src/utils/integration-config.ts`), the Slack bot when the requesting user has no saved preference (`packages/slack-bot/src/user-preferences.ts`), and the Linear bot when neither repo config, user preference, nor a `model:` issue label selects one (`packages/linear-bot/src/model-resolution.ts`). No bot TypeScript changes: the binding name and its meaning are unchanged. ## Validation Modelled on the existing `classification_model` variable, including its "a prefix with nothing after it names no model" reasoning. Accepted shapes match what `normalizeModelId` in `packages/shared/src/models.ts` actually resolves — a canonical `provider/model` id, or a bare `claude-`/`gpt-` id that the bots normalize into `anthropic/`/`openai/`. A value with an empty side (`anthropic/`, `claude-`, `/x`) is rejected, as is a bare id with no recognized prefix (`haiku-4-5`). That rule also rejects blank, which matters for CI: an unset Actions variable renders as an empty string, and treating that as "use the default" would silently deploy whichever model the configuration last shipped. It now fails at plan time instead. Unlike `classification_model`, the accepted namespace is not restricted to Anthropic and OpenAI — `MODEL_CATALOG` also ships `xai/`, `opencode/`, `deepseek/` and `zai-coding-plan/` ids, and a `DEFAULT_MODEL` is a session model, not a classifier model with a provider-specific credential requirement. ## CI `TF_VAR_github_bot_default_model`, `TF_VAR_slack_bot_default_model` and `TF_VAR_linear_bot_default_model` are threaded through both the `plan` and `apply` jobs of `.github/workflows/terraform.yml`, using the existing `${{ vars.X || secrets.X || 'default' }}` precedence with an explicit fallback so an unset variable does not render as a blank that fails validation. The GitHub one is named `GH_BOT_DEFAULT_MODEL` because Actions reserves the `GITHUB_` prefix, matching the existing `GH_BOT_USERNAME`. ## Upstream-structure adaptation `terraform/modules/cloudflare-worker/outputs.tf` gains a `plain_text_bindings` `name => value` map output. The module previously exposed only `plain_text_binding_names`, which lets a test assert that a binding exists but not what it carries — so "the default still reaches `DEFAULT_MODEL`" was not assertable. The new output sits next to the existing name-list outputs and exposes nothing sensitive (plain-text bindings are non-secret by construction; `secrets` remains name-only). ## Gates run locally In `terraform/environments/production`, Terraform v1.15.3: - `terraform validate` — `Success! The configuration is valid.` - `terraform fmt -check -recursive` on `terraform/environments/production` and `terraform/modules/cloudflare-worker` — clean, no reformatting of untouched files. - `terraform test` — `Success! 42 passed, 0 failed.` (34 pre-existing + 8 new) The new `tests/bot_default_model.tftest.hcl` enables all three bots and covers: - `defaults_preserve_the_previously_hardcoded_models` — each bot's `DEFAULT_MODEL` binding still carries the exact literal it carried before this change. - `overrides_reach_each_bot_binding` — an override of each variable reaches that bot's binding, and no bot's override leaks into another's. - six validation cases — blank on each of the three variables, a bare provider namespace, a bare model prefix, and an unprefixed bare id. Falsification check that the new test actually bites: repointing the Slack worker's binding at `var.linear_bot_default_model` fails three assertions across both plan runs (`6 passed, 2 failed`); reverted before commit. PRs from a fork land as `action_required` here — upstream CI awaits maintainer approval — so the above are local runs rather than a green check. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added configurable default model settings for the GitHub, Slack, and Linear bots. * Supports provider/model identifiers and approved shorthand model IDs. * Existing default model selections remain unchanged unless overridden. * **Documentation** * Added example configuration entries describing each bot’s default model setting. * **Bug Fixes** * Invalid, blank, or incomplete model identifiers are now rejected during configuration validation. * Invalid whitespace and multi-provider model identifiers are also rejected. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…awn staleness bound to it (ColeMurray#1821) Two related reliability defects in sandbox spawn/connect handling. ## 1. The initial-connect watchdog was too tight `DEFAULT_CONNECTING_TIMEOUT_CONFIG.timeoutMs` gave a sandbox two minutes to report connected. The boot sequence (git clone, setup.sh, start.sh, opencode, bridge connect) is 30-90 seconds for a small repo, but a large repo with a real setup script lands right at that limit. Overrunning it is not a soft failure. `handleAlarm` marks the sandbox failed and calls `clearSandboxAccessState`, which locks out the sandbox that does eventually come up; the queued prompt is never re-driven; and the documented recovery ("it will be retried on your next message") cannot fire for bot-triggered sessions, which only ever send one prompt. So a boot that overran by a few seconds stranded its session permanently and lost the review with no signal on the PR. This widens the bound to four minutes so slow-but-healthy boots have real headroom. It is a mitigation of the symptom, not a fix for the underlying recovery gap, which is ColeMurray#1363 - a session that trips the watchdog should be recoverable rather than terminal, and that is a bigger change than this. ## 2. The staleness bound and the watchdog could disagree `DEFAULT_SPAWN_CONFIG.spawningTimeoutMs` decides when a sandbox sitting in `spawning`/`connecting` is stale enough that `evaluateSpawnDecision` stops skipping and lets a replacement spawn. Its comment said it matched the connecting-timeout watchdog, but the value was restated independently as a second literal. That made the two silently separable, and widening the watchdog in (1) alone would have opened a two-minute window in which a healthy sandbox still inside the watchdog window is judged dead and a second sandbox is spawned alongside it - a duplicate provider sandbox per slow boot. Both now derive from a single `CONNECT_WATCHDOG_MS`, so the invariant holds by construction rather than by comment, and the rationale for the value lives in one place. ## Why this is generally useful Neither part is deployment-specific: no new configuration, no new environment variable, no provider-specific behavior. The first is a defaults change that only affects how much slack a slow boot gets before it is declared dead; the second removes a duplicated constant whose two copies had already drifted in meaning, which is the kind of coupling that reappears the next time either number is touched. ## Tests `evaluateConnectingTimeout` tests that hardcoded offsets against the old two-minute value now derive them from `config.timeoutMs`, and the same in `manager.test.ts`, so they assert the behavior rather than the constant. Two new tests in `decisions.test.ts` (`connect watchdog and spawn staleness defaults`) exercise the shipped defaults together: a sandbox one millisecond inside the watchdog window must not get a replacement spawn, and once the watchdog has failed it, it must. Falsified by reintroducing the old independent literal `spawningTimeoutMs: 120_000`, which fails the first of the two (`expected 'spawn' to be 'skip'`); restored, both pass. ## Gates run locally Run in a clean worktree off `main` at `265a5997`: - `npm run build -w @open-inspect/shared` - exit 0 - `npm run typecheck -w @open-inspect/control-plane` (all four TS projects: default, `tsconfig.node.json`, `tsconfig.test.json`, `test/integration`) - exit 0 - `npm test -w @open-inspect/control-plane` - 275 test files passed, 4011 tests passed, exit 0 - `npm run test:integration -w @open-inspect/control-plane` - 102 test files passed, 1186 tests passed, 1 skipped, exit 0 No other package is touched, so package-scoped gates only. ## Adaptation to current main The change was originally written against an older revision of this file. One test it edited, `calls onSandboxTerminating callback on connecting timeout` in `manager.test.ts`, no longer exists on `main` - that area is now the `terminateUnresponsiveSandbox` describe block - so only the surviving `detects connecting timeout and sets failed` case needed the derived offset. Nothing upstream was reverted and no upstream test or assertion was removed. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Extended the sandbox connection and startup watchdog window to four minutes. * Sandboxes still within the connection window are now skipped, while replacements are spawned after the watchdog expires. * Updated timeout handling to use the configured watchdog duration consistently. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…oup across 1 directory (ColeMurray#1843) Bumps the npm_and_yarn group with 1 update in the / directory: [nanoid](https://github.com/ai/nanoid). Updates `nanoid` from 3.3.17 to 3.3.18 <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/ai/nanoid/releases">nanoid's releases</a>.</em></p> <blockquote> <h2>3.3.18</h2> <ul> <li>Fixed infinite loop on async for React Native (by <a href="https://github.com/OvergrowthBeards-JB"><code>@OvergrowthBeards-JB</code></a>).</li> </ul> </blockquote> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/ai/nanoid/blob/3.3.18/CHANGELOG.md">nanoid's changelog</a>.</em></p> <blockquote> <h2>3.3.18</h2> <ul> <li>Fixed infinite loop on async for React Native (by <a href="https://github.com/OvergrowthBeards-JB"><code>@OvergrowthBeards-JB</code></a>).</li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/ai/nanoid/commit/9ad98052b316c5e707f8098ace509d2ae165e54d"><code>9ad9805</code></a> Release 3.3.18 version</li> <li><a href="https://github.com/ai/nanoid/commit/55e50a0621ec084b4bb4000ea4e86e1191bd3da8"><code>55e50a0</code></a> Update CI action</li> <li><a href="https://github.com/ai/nanoid/commit/e10f8d40ce9d1ab47f66d65a16b48086432730d0"><code>e10f8d4</code></a> Update index.native.js (<a href="https://redirect.github.com/ai/nanoid/issues/606">#606</a>)</li> <li>See full diff in <a href="https://github.com/ai/nanoid/compare/3.3.17...3.3.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>
Terraform Validation Results
Pushed by: @jasoncuriano, Action: |
ScottKirschner
approved these changes
Sep 9, 2026
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
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.
Routine upstream sync per
upstream-sync-runbook.md. Brings the fork up to30b885b5.upstream/main— no fork-local changespackage-lock.jsonuntouched (zero version/resolved/integrity changes)Verified locally before push:
npm run build(all packages)npm run typechecknpm test -w @open-inspect/control-planedist/index.js× 5Note: five new
@open-inspect/sharedsubpath exports landed (classification,rbac,pull-request-tool,types/github-autofix,types/audit-events) — a cleansharedrebuild is required, per the runbook.
🤖 Generated with Claude Code