chore: sync fork with upstream (ColeMurray/background-agents) - #17
Merged
Conversation
## Summary - define the canonical GitHub and Google issuer mapping in `@open-inspect/shared` - persist `provider_issuer` from both `UserStore` identity insertion paths while leaving Slack and Linear issuers null - expose the stored issuer through `UserIdentity` and reuse the shared constants in sign-in provider code - add D1 migration `0056` to backfill GitHub and Google identities created with null issuers - cover both identity creation paths for all four providers ## Verification - `npm run build -w @open-inspect/shared` - `npm run build -w @open-inspect/control-plane` - `npm run typecheck -w @open-inspect/shared` - `npm run typecheck -w @open-inspect/control-plane` - `npm test -w @open-inspect/shared -- --run src/sign-in-provider.test.ts` - `npm test -w @open-inspect/control-plane` (153 files, 2,311 tests) - `npm run test:integration -w @open-inspect/control-plane -- test/integration/user-store.test.ts` (24 tests) - isolated reruns of `websocket-client.test.ts` and `session-diffs.test.ts` passed after load-related timeouts in the full parallel integration run - ESLint, Prettier, and `git diff --check` Closes COL-27. --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/4b4921ea11a733b1f6939800648b76a3)* <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Improvements** * Added canonical issuer tracking for user identities across supported sign-in providers. * Improved GitHub and Google sign-in validation consistency. * Existing GitHub and Google identities are automatically populated with provider issuer information. * **Reliability** * Added validation to ensure issuer data is consistently mapped and persisted across sign-in providers. * Added safeguards to preserve existing issuer values and leave unsupported provider records unchanged. <!-- 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 - omit the redundant legacy `openinspect_environment` tag from Vercel image-build sandbox requests - preserve the shared legacy label for providers without Vercel's five-tag restriction - assert the exact five-tag Vercel image-build payload in the provider test ## Context Commit `a22e3fc` expanded image-build identity labels from four to six. Vercel's `POST /v2/sandboxes` API permits at most five tags, causing Vercel image-build sandbox creation to fail validation. ## Verification - `npm test -w @open-inspect/control-plane -- --run src/sandbox/providers/vercel/provider.test.ts` - `npm run typecheck -w @open-inspect/control-plane` - `npx eslint packages/control-plane/src/sandbox/providers/vercel/provider.ts packages/control-plane/src/sandbox/providers/vercel/provider.test.ts` - `npm test -w @open-inspect/control-plane` (154 files, 2312 tests) - independent sub-agent review: no findings --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/897b078a445296058a8b195ecd56c7ea)* --------- Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com>
…oleMurray#1300) Fixes ColeMurray#1290. ## Problem After the Better Auth cutover (ColeMurray#1126, migrations 0047–0049), the system ran two disjoint identity registries sharing one id space: canonical `users`/`user_identities` (written by bot ingress) and Better Auth's `auth_users`/`auth_accounts` (written at web sign-in). Nothing bridged them: canonical users were locked out of web sign-in, bot-first users kept hitting the same wall, and users split bidirectionally into phantom rows. ## Change: consolidation, not synchronization Per maintainer direction, this PR **dissolves the parallel registry** instead of bridging it. Better Auth now persists directly into the canonical tables through a custom adapter — its user model IS `users`, its account model IS `user_identities`. A bot-created GitHub identity is an account, so `findOAuthUser`'s account-first lookup signs bot-first users straight into their canonical row. The entire cross-registry drift class (stranded graphs, phantom splits, missing projections) ceases to exist structurally. ### Custom adapter (`db/better-auth-adapter.ts`) `createAdapterFactory` adapter on the `SqlDatabase` seam: generic SQL executor (8 methods, all 11 where-operators), Date ⇄ epoch-ms transforms, boolean ⇄ 0/1, `provider_issuer` injection on account creates, blank `display_name` folding. Ids stay canonical 32-hex via the existing `generateId` config. `encryptOAuthTokens` continues to run above the adapter, so token columns hold ciphertext. ### Migration 0057 — one-time fold-in, then drop - `users` gains `email_verified` (the implicit-linking gate, `requireLocalEmailVerified`); `user_identities` gains the OAuth credential columns + `updated_at`. - Same-id auth rows merge into their canonical row (email/profile backfill, guarded + `OR IGNORE` nets so no drift state can abort the Terraform apply). - Web-only auth rows become canonical users; their accounts fold in with credentials. - One-time reviewed backlog verify: every emailed canonical user existing at cutover gets `email_verified = 1` — this is what unlocks the ColeMurray#1290 lockout cohorts. Post-cutover, verification is minted only from OAuth proof at sign-in. - Strands whose email belongs to a different canonical user are superseded (preflight-counted): their next sign-in email-links onto the row that owns their history. - `auth_sessions` re-keys onto `users` (epoch-ms columns); `auth_verifications` recreated (ephemeral state); `auth_users`/`auth_accounts` dropped. Rollback for the cutover window is the preflight D1 Time Travel bookmark (operator runbook in the migration header). ### Claim decorator (`auth/user/sign-in-claim.ts`) What no framework can do: trust bot-attributed data. The OAuth callback is the one moment a provider-verified email is in hand, so a small decorator around `getUserInfo` backfills NULL emails, normalizes legacy email forms, and mints `email_verified = 1` from the proof — swallow-and-log, never failing a sign-in. Divergent multi-surface pairs are evented (`auth.subject_email_collision`) and the sign-in lands account-first on the subject owner; the merge script converges the pair. ### Deleted `CanonicalUserProjection`, `AccountIdentityProjection` + all four databaseHooks, the two-tier reconciliation decorator, the R1–R5 consistency reports, the scheduled reconciliation job, and 0057's former runtime sweep. The merge script drops its entire auth-graph half (email parking, surviving-email selection) — browser sessions simply re-point. **Net −1,914 lines.** ## Behavior changes vs. the bridged design - Slack/Linear ingress writes `email_verified = 1` (per-maintainer decision): both platforms verify mailbox ownership (Slack confirms address changes, Linear's email is its login credential) and the first-party bots fetch the address server-side, so attribution from these providers carries the same weight as OAuth proof. The attestation is an explicit per-provider set (`EMAIL_ATTESTING_PROVIDERS`) — a future ingress provider stays unproven unless deliberately added. - Bot-first users with an unproven attributed email (non-attesting providers, legacy rows) still heal at first sign-in — the incoming OAuth proof of exactly that email mints verification instead of the linking gate refusing them. - A shared-subject/email split signs into the **subject owner** (the row with their history), not the email owner — account-first is now the natural priority. The pair remains enumerable merge work. - `user_identities` is now the live credential store for browser-linked GitHub/Google tokens (read via `auth.api.getAccessToken`). Better Auth's ciphertext (keyed by `BROWSER_AUTH_SECRET`) coexists with the app's `TOKEN_ENCRYPTION_KEY` domain used elsewhere — documented, deliberate. - D1 has no interactive transactions, so the adapter runs statements sequentially (`transaction: false`); with one registry and client-generated ids, a mid-register failure self-heals at the next sign-in instead of stranding a parallel-table graph. ## Testing - 9 migration fold-in tests (pre-cutover schema reconstructed in-test, real 0057 statements executed: merges, web-only creation, strand supersession, backlog verify, credential grafting incl. the cross-owner refusal, session re-keying, whitespace variants, unparseable timestamps) - 11 end-to-end sign-in flows through the real worker + workerd D1 + real Better Auth (every cohort, the collision case, legacy email normalization, cross-provider linking, credential storage) - 7 merge tests, updated browser-auth/callback suites, schema-alignment assertions for the adapter's field maps - Suites: 2,312 unit + 741 integration green; typecheck/lint clean; CLI smoke-verified ## Rollout Single deploy: Terraform applies 0057 before the worker. Capture the Time Travel bookmark and preflight counts (migration header) first. In-flight OAuth handshakes at the deploy moment fail once and retry; sessions of superseded strands end (by design). Watch `auth.subject_email_collision` (enumerable merge work) and `auth.claim_failed` (should be ~never). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added safer GitHub and Google sign-in account linking with verified-email claiming and collision protection. * Added duplicate user account merging with preview, resumable execution, and validation. * Added a command-line utility for previewing and executing user merges. * **Improvements** * Consolidated authentication records into the primary user and identity system. * Normalized email addresses consistently; blank values are treated as absent. * Preserved OAuth credentials, sessions, verification status, and related data during migration. * **Documentation** * Clarified the separate encryption keys used for browser authentication and SCM tokens. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…Murray#1237) **Prevents persisted terminal state from producing a server/client hydration mismatch and gives grouped settings controls accurate accessible structure.** ## Root-cause tasks Fixed **10 root-cause tasks**: 1. `react-doctor/no-hydration-branch-on-browser-global` - `packages/web/src/app/(app)/session/[id]/page.tsx` - terminal visibility read `localStorage` during initial render. The preference now uses `useSyncExternalStore` with a stable server snapshot, preventing hydration drift and inconsistent terminal rendering. **Human severity: high.** 2. `react-doctor/rerender-lazy-ref-init` - `packages/web/src/hooks/use-session-attachments.ts` - an upload cache `Map` was allocated on every render and discarded after the first. Lazy initialization avoids unnecessary allocations in the attachment composer. **Human severity: low.** 3. `react-doctor/rerender-lazy-ref-init` - `packages/web/src/hooks/use-session-participant-profiles.ts` - the attempted-profile ID `Set` was reallocated on every render. Lazy initialization reduces avoidable work during participant updates. **Human severity: low.** 4. `react-doctor/rerender-lazy-ref-init` - `packages/web/src/hooks/use-session-socket.ts` - the subscription waiter `Set` was reallocated on every socket-hook render. Lazy initialization avoids churn on a high-activity session path. **Human severity: low.** 5. `react-doctor/label-has-associated-control` - `packages/web/src/components/automations/automation-form.tsx` - the trigger-type group heading was an unassociated form label. It is now neutral text, avoiding misleading label semantics for assistive technology. **Human severity: medium.** 6. `react-doctor/label-has-associated-control` - `packages/web/src/components/automations/automation-form.tsx` - the repository configuration label did not identify its popover trigger. Matching `htmlFor`/`id` values now associate the visible label with the control. **Human severity: medium.** 7. `react-doctor/label-has-associated-control` - `packages/web/src/components/settings/sandbox-settings.tsx` - Service Ports was an unassociated label for two controls. A `fieldset`/`legend` now exposes the controls as one named group. **Human severity: medium.** 8. `react-doctor/label-has-associated-control` - `packages/web/src/components/settings/sandbox-settings.tsx` - Tunnel Ports was an unassociated label for a dynamic control group. A `fieldset`/`legend` now provides group semantics without changing the visible heading. **Human severity: medium.** 9. `react-doctor/label-has-associated-control` - `packages/web/src/components/settings/sandbox-settings.tsx` - Child Sessions was an unassociated label for two limits. A `fieldset`/`legend` now gives screen-reader users the shared context. **Human severity: medium.** 10. `react-doctor/label-has-associated-control` - `packages/web/src/components/settings/sandbox-settings.tsx` - Resources was an unassociated label for CPU and memory controls. A `fieldset`/`legend` now names that control group. **Human severity: medium.** All selected diagnostics were ungrouped. No non-null `fixGroupId` was selected or split. Diagnostics with a shared `fixGroupId` remain deferred as complete groups. ## React Doctor results - Total diagnostics: **108 before, 98 after** - Errors: **2 before, 1 after** - Warnings: **106 before, 97 after** - `no-hydration-branch-on-browser-global`: **1 → 0** - `rerender-lazy-ref-init`: **3 → 0** - `label-has-associated-control`: **13 → 7** - Raw diagnostics cleared: **10** - No new React Doctor rule sites were introduced. ## Validation - `npm run typecheck -w @open-inspect/web` after the error fix and after each warning task: passed - `npm run typecheck -w @open-inspect/web`: passed - `npm run lint -w @open-inspect/web`: passed - `npx prettier --check packages/web`: passed - `npm test -w @open-inspect/web`: passed, 111 files and 852 tests - Focused automation, sandbox settings, attachment, participant-profile, and socket tests: passed, 5 files and 100 tests - Full React Doctor after-scan: passed, 98 diagnostics - Changed-scope React Doctor scan against `origin/main`: no new finding; it reports the pre-existing `prefer-useReducer` warning at `sandbox-settings.tsx:234` - `npm run build -w @open-inspect/web`: still fails at the same pre-existing `/automations/new` prerender with `Cannot read properties of null (reading 'useContext')`, digest `2120487162`; compilation and TypeScript complete successfully before that failure ## Existing failures - The production build failure above was reproduced before editing and remains identical after the changes. - The repository-wide formatter baseline reports pre-existing formatting in `.opencode/package.json`; the web-only formatting check passes. - The remaining `effect-needs-cleanup` error is a validated false positive: the effect calls a callback that creates the WebSocket, and the effect's returned teardown closes that same socket, matching the canonical rule exclusion. ## Deferred - Broad component splitting (`no-giant-component`) and reducer migrations remain for later code-owner-reviewed batches. - Grouped prop/state adjustment findings were left intact; no `fixGroupId` was split. - Sensitive integration/auth settings and terminal iframe sandbox policy require human judgment. - Locale presentation, placeholder labeling, image optimization, dynamic chart imports, array-key identity, and migration-scale iteration/lookup changes remain deferred where runtime or UX intent is unclear. - Remaining custom-control label findings require component-level accessibility decisions rather than mechanical edits. ## Visual verification Browser visual verification was not run. The UI edits are intended to be visually neutral semantic changes, and the terminal change affects hydration/state synchronization rather than layout; lint, typecheck, and focused interaction tests cover the touched behavior. --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/ec8ab60a274d07681a343550a3343bf6)* --------- Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com> Co-authored-by: Cole Murray <colemurray.cs@gmail.com>
…ray#1256) ## Summary Fixes **5 root-cause tasks** identified by the nightly React Doctor scan. 1. **High - `react-doctor/no-hydration-branch-on-browser-global` - `packages/web/src/app/(app)/session/[id]/page.tsx`** The terminal panel initialized from `localStorage` during render, so server and first-client output could disagree. Terminal visibility now uses `useSyncExternalStore` with a stable server snapshot and same-tab/cross-tab updates, preventing hydration mismatches without introducing a post-paint flicker. 2. **Low - `react-doctor/only-export-components` - `packages/web/src/components/session-list-item.tsx`** `buildSessionHref` shared a component module, invalidating the Fast Refresh boundary. Moving it to `session-list-item-utils.ts` lets developers retain component state during edits. 3. **Low - `react-doctor/only-export-components` - `packages/web/src/components/session-timeline.tsx`** Internal event grouping helpers were unnecessarily exported from a component module. Keeping them module-private restores a safe Fast Refresh boundary without changing runtime behavior. 4. **Low - `react-doctor/only-export-components` - `packages/web/src/components/settings/image-build-status.tsx`** The ready-details formatter shared a component module. Moving it to `image-build-status-utils.ts` preserves Fast Refresh state for image status UI development. 5. **Low - `react-doctor/only-export-components` - `packages/web/src/components/terminal-message-read-observer.tsx`** The message-read decision helper shared a component module. Moving the helper and its single attempt-limit constant to `terminal-message-read-state.ts` keeps the component boundary refresh-safe and preserves test coverage. All selected diagnostics were ungrouped and therefore counted individually. No non-null `fixGroupId` was selected or split; grouped findings remain intact for later review. ## React Doctor - Scanner: React Doctor `0.9.3`, schema v3, full scope - Before: **108 diagnostics** (2 errors, 106 warnings), score 59 - After: **103 diagnostics** (1 error, 102 warnings), score 63 - Raw diagnostics cleared: **5** - `no-hydration-branch-on-browser-global`: 1 -> 0 - `only-export-components`: 6 -> 2 - Changed-scope regression scan: no issues found, score 87 - No new full-scan diagnostics were introduced ## Validation - `npm run typecheck` in `packages/web`: passed after the error-severity fix and after final implementation - `npx vitest run src/components/session-sidebar.test.tsx src/components/session-timeline.test.tsx src/components/settings/images-settings.test.tsx src/components/terminal-message-read-observer.test.ts`: 29 passed - `npm test -w @open-inspect/web`: 114 files, 869 tests passed - `npm run lint` in `packages/web`: passed - `npx prettier --check packages/web`: passed - `npx -y react-doctor@0.9.3 . --json --json-out /tmp/react-doctor-after.json --yes --blocking none`: complete, selected tasks gone - `npx -y react-doctor@0.9.3 . --verbose --scope changed --base origin/main --yes --blocking none`: no issues found - `npm run build` in `packages/web`: compiled and typechecked, then hit the pre-existing `/automations/new` prerender failure described below ## Pre-existing Failures - The initial full test baseline timed out once in `src/lib/site-config.test.ts:30`; the final full suite passed all 869 tests. - The production build fails while prerendering `/automations/new` with `TypeError: Cannot read properties of null (reading 'useContext')` and emits generated-page missing-key warnings. The exact `origin/main` commit reproduces the same failure and warnings in an isolated worktree, so this PR adds no build regression. ## Deferred - `effect-needs-cleanup` in `use-session-transport.ts` is a canonical false positive: the mount effect closes the WebSocket, invalidates in-flight opens, and clears reconnect timers. No suppression was added. - The remaining two `only-export-components` findings in shared button/toggle variant modules were left for a later batch to keep this PR focused. - Findings requiring stable product IDs, rendered accessibility evidence, timezone/locale policy, image-host behavior, performance measurement, state architecture changes, security decisions, or broad component decomposition were deferred for code-owner judgment. - Migration-scale families such as state/effect rewrites, giant components, list keys, labels, image optimization, and iteration micro-optimizations remain explicitly out of scope. ## Visual Verification Not run because this change does not alter UI markup, styling, or intended visual behavior. The terminal persistence change preserves the existing visible state while making hydration deterministic. --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/1488a665176224d47719cd7ba5609642)* Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com> Co-authored-by: Cole Murray <colemurray.cs@gmail.com>
<!-- react-doctor-owner: open-inspect-nightly --> <!-- react-doctor-bucket: web-nightly-2026-08-04 --> <!-- react-doctor-base-sha: 23754a2 --> <!-- react-doctor-diagnostics: 10 --> ## Summary Fixes **10 root-cause task units** from the nightly React Doctor scan in `packages/web`. 1. **High** - `react-doctor/no-hydration-branch-on-browser-global` in `src/app/(app)/session/[id]/page.tsx`: terminal visibility read `localStorage` during render, so the server and hydrating client could choose different UI. The terminal preference now uses `useSyncExternalStore` with a stable server snapshot, preventing hydration errors while preserving same-tab and cross-tab updates. 2. **Medium** - `react-doctor/label-has-associated-control` in `src/components/automations/automation-form.tsx` (create-mode Trigger Type): a `<label>` acted as a section heading without naming one control. It now uses neutral heading text, avoiding misleading form semantics for assistive technology. 3. **Medium** - `react-doctor/label-has-associated-control` in `src/components/automations/automation-form.tsx` (read-only Trigger Type): a `<label>` described display-only content. It now uses neutral text so screen readers do not announce an unattached form label. 4. **Medium** - `react-doctor/label-has-associated-control` in `src/components/automations/automation-form.tsx` (Repository Configuration): a group heading was represented as an unattached label. Neutral markup preserves appearance while removing invalid form semantics. 5. **Medium** - `react-doctor/label-has-associated-control` in `src/components/automations/automation-form.tsx` (Conditions): a group heading was represented as an unattached label. Neutral markup avoids an inaccessible label/control relationship. 6. **Medium** - `react-doctor/label-has-associated-control` in `src/components/settings/sandbox-settings.tsx` (Service Ports): a multi-field section heading was an unattached label. Neutral markup keeps individual port labels authoritative. 7. **Medium** - `react-doctor/label-has-associated-control` in `src/components/settings/sandbox-settings.tsx` (Tunnel Ports): a dynamic-list heading was an unattached label. Neutral markup prevents misleading form semantics. 8. **Medium** - `react-doctor/label-has-associated-control` in `src/components/settings/sandbox-settings.tsx` (Child Sessions): a two-field group heading was an unattached label. Neutral markup leaves each input's associated label intact. 9. **Medium** - `react-doctor/label-has-associated-control` in `src/components/settings/sandbox-settings.tsx` (Resources): a two-field group heading was an unattached label. Neutral markup leaves the CPU and memory labels intact. 10. **Medium** - `react-doctor/no-array-index-as-key` in `src/components/tool-call-group.tsx`: expanded tool calls used their array index in React keys, which could transfer row state when events changed order. The existing stable `toolCallKey(event)` now identifies each row. All selected diagnostics were ungrouped (`fixGroupId: null`), so each diagnostic counted as one task unit. No non-null `fixGroupId` was split or included. ## Scan Results React Doctor v0.9.4, schema v3, full `packages/web` scope: - Before: **99** diagnostics, **2 errors / 97 warnings**, score 59 - After: **89** diagnostics, **1 error / 88 warnings**, score 64 - Raw diagnostics cleared: **10 net** - `no-hydration-branch-on-browser-global`: 1 -> 0 - `label-has-associated-control`: 13 -> 5 - `no-array-index-as-key`: 8 -> 7 - No new semantic finding was introduced. The same pre-existing `no-giant-component` finding on `SessionPageContent` received a new line-based diagnostic ID after lines were added. The changed-scope regression scan reports only the pre-existing `prefer-useReducer` warning in `sandbox-settings.tsx`, already present in the full baseline. ## Validation - `npm run typecheck` after the error-severity fix: passed - `npm run typecheck`: passed - `npx prettier --check packages/web`: passed - `npm run lint`: passed - `npm test -- src/components/automations/automation-form.test.tsx src/components/settings/sandbox-settings.test.tsx`: passed, 71 tests - `npm test`: baseline and final runs retain intermittent pre-existing 5-second timeout failures. Baseline: 3 timeouts across the two auth-boundary ESLint tests and Slack routing-limit test. Last final run: only the Slack routing-limit timeout remained; 875/876 tests passed. - `npm run build`: reproduces the baseline `/automations/new` prerender failure, `TypeError: Cannot read properties of null (reading 'useContext')`; compilation and TypeScript complete first. - `npx -y react-doctor@latest . --json --json-out /tmp/react-doctor-after.json --yes --blocking none`: valid complete scan; all selected tasks absent - `npx -y react-doctor@latest . --verbose --scope changed --base origin/main --yes --blocking none`: no introduced diagnostic; one pre-existing touched-file warning No visual verification was needed: the semantic markup changes retain identical classes and appearance, and the terminal change preserves existing UI states while correcting hydration and subscription behavior. ## Deferred - `effect-needs-cleanup` in `use-session-transport.ts` is a detector false positive: the socket is created by a callback invoked from a mount effect, whose existing cleanup invalidates in-flight connection work, clears the reconnect timer, and closes the exact socket. No suppression was added. - Migration-scale component splitting, reducer/state-effect rewrites, image migration, locale/timezone decisions, and dynamic-import work remain for later review. - Custom-control labeling findings that need component API or grouping decisions remain deferred. - The iframe sandbox finding remains deferred because changing iframe permissions requires a product/security review. - Remaining index-key findings without a proven stable identity and performance findings without runtime measurements remain deferred. --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/9d2b2c3259ea29fcc813e5da628f5c18)* --------- Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com> Co-authored-by: Cole Murray <colemurray.cs@gmail.com>
## Summary - Add coverage for create-time image-build callback URL validation so mixed control-plane and external callback URLs are rejected before provider sandbox creation. - Add coverage for independent clamping of `build_execution_timeout_seconds` from the outer build sandbox timeout. ## Why Recent Modal image-build lifecycle changes route callbacks and timeout budgets through `api_create_build_sandbox`, which is core business logic for provider-session safety. These tests lock down important validation behavior without changing production code. ## Testing - `uv run pytest tests/ -q` from `packages/modal-infra` --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/36c86fd3d901c721990bb29948bf6389)* Co-authored-by: waclaude <colemurray.cs+ghwaclaude@gmail.com>
…ColeMurray#1291) ## Summary An agent posting through the notify route has its message cut at 2900 characters, mid-sentence. The response reports `truncated: true`, so the agent finds out — after the fact, with the text already gone and no way to recover it. The cut lands on the tail, which for any analysis writeup is where the conclusions and recommendations live. Observed in practice: an agent posted a long investigation into a Slack thread, and the message ended `1. No p…`. Everything after that — the entire "what to do about it" section, the only part the reader needed — was discarded. The agent then had to guess what had been lost and post a follow-up. **The completion path already solved this.** `splitIntoSlackSections` spreads long text across consecutive section blocks and keeps code fences balanced across the seam, so a long answer arrives whole. The notify route never got the same treatment — the same worker had two different answers to the same Slack limit. This moves the splitter into `@open-inspect/shared/slack` and uses it in both places: - `slack-notify` sanitizes against `RAW_TEXT_INPUT_MAX_LENGTH` (the cap it already validates and enforces), splits the result, and emits one section block per chunk. `SLACK_TEXT_MAX_LENGTH` is gone. - The splitter's ceiling (`MAX_RESPONSE_SECTIONS` × `SECTION_TEXT_MAX_CHARS` = 60k) sits far above that 12k input cap, so `truncated` in the response now reflects only the documented raw-input limit rather than a Slack rendering detail. - The completion path is unchanged: it imports the same function from shared instead of defining it locally. ## Notes The splitter's tests deliberately stay in `packages/slack-bot/src/completion/blocks.test.ts`. They exercise it *through* `buildCompletionBlocks`, so leaving them in place also asserts that moving the function left the completion output byte-identical. Moving them to a shared-level suite would have lost that guarantee for no gain. The fallback text passed to `chat.postMessage` is now the opening section rather than the whole body — a section-split message has no single text body, and Slack only uses that field for the notification preview. Worth a separate look: the `slack-notify` tool description doesn't mention any length behaviour, so an agent has no way to budget for it. Now that long messages split cleanly that matters less, but stating it would let the model structure long output deliberately rather than discovering the limit from a response flag. ## Test plan - [x] `npm test -w @open-inspect/shared` (545), `-w @open-inspect/slack-bot` (406), `-w @open-inspect/control-plane` (2312) - [x] `npm run typecheck`, `npm run lint` clean - [x] New regression test in `src/routes/slack-notify.test.ts`: a message well over one section keeps every section within 3000 chars, emits more than one section, reports `truncated: false`, and — the point of the test — still contains its closing recommendation. It fails on `main`, where that tail is cut. - [x] The existing 406 slack-bot tests pass unchanged, confirming the completion path is unaffected by the move. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Slack notifications now split long responses into multiple sections without losing the final recommendation. * Each section respects Slack’s 3,000-character limit. * Responses preserve paragraph, line, Unicode, and code-block formatting where possible. * Extremely long responses are capped with a truncation indicator after 20 sections. * **Tests** * Added regression coverage for oversized Slack messages, section limits, and truncation behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary - include the existing valid callback URL context in the build execution timeout clamping test - keep the fix scoped to the stale test fixture; production behavior is unchanged ## Root cause PR ColeMurray#1255 was validated on August 3 before callback URLs became required for build sandbox creation on August 4. It was merged on August 6 without current-base validation, so the newly added timeout test reached main with an outdated request fixture. The test failed during callback_url validation before exercising either timeout assertion. Fixes the CI failure in https://github.com/ColeMurray/background-agents/actions/runs/31133401945/job/92727432767. ## Validation - uv run pytest tests/ -q (177 passed) - uv run ruff check tests/test_web_api_build_sandbox.py - uv run ruff format --check tests/test_web_api_build_sandbox.py - git diff --check <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Updated build-execution-timeout coverage to include standard callback URL fields in the request payload. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary - let Slack derive accessible fallback text from every notification block - preserve the full content of section-split agent notifications for screen readers - align agent notify delivery with the existing completion-delivery accessibility pattern ## Root cause PR ColeMurray#1291 split long agent notifications into multiple blocks but supplied only the opening section as top-level `text`. Slack screen readers prefer that field over interior blocks, so later sections remained visually available while being omitted from the accessible representation. This uses the existing `postBlocks` client so Slack derives fallback text from the supported blocks instead. ## Test plan - `npm run build -w @open-inspect/shared` - `npm test -w @open-inspect/control-plane -- src/routes/slack-notify.test.ts` - `npm test -w @open-inspect/control-plane` (2,313 tests) - `npm run typecheck -w @open-inspect/control-plane` - `npm run lint -w @open-inspect/control-plane` - `npm exec prettier -- --check packages/control-plane/src/routes/slack-notify.ts packages/control-plane/src/routes/slack-notify.test.ts` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved Slack notifications for long messages by preserving all content without truncation. * Enhanced accessibility by allowing screen readers to derive notification text from message sections. * Maintained message sanitization and threaded notification behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Removes the transitional `build_timeout_seconds` → `provider_session_timeout_seconds` rename-skew shim introduced in ColeMurray#1288, now that the rename is fully deployed on both planes: - **control plane** (`sandbox/client.ts`): stop dual-writing the legacy `build_timeout_seconds` key alongside `provider_session_timeout_seconds` in the create-build-sandbox payload. - **Modal** (`web_api.py`): drop the `legacy_field="build_timeout_seconds"` dual-read alias and remove the now-unused `legacy_field` parameter from `_validated_timeout_seconds`. - Tests: the two rename-skew tests are replaced by a single test pinning that the retired key is now **ignored** (falls back to the default provider-session timeout), so an accidental revival of the old key would be caught. Both shim sites carried "drop once both planes are past the rename" comments; this is that follow-up. The rename (ColeMurray#1288) has merged, synced to the production fork, and deployed on both pipelines, so the currently-deployed control plane already sends the new key and the currently-deployed Modal prefers it — removal is safe in either deploy-skew direction. ## Also fixes a red test on main `test_create_clamps_build_execution_timeout_independently` (added in ColeMurray#1255) was failing on `main`: its payload predates ColeMurray#1283 making callback URLs required at create, so the endpoint 400s before reaching the timeout logic. Since this PR rewrites that test's payload anyway (it used the retired key), it also adds the required `**CALLBACK_CONTEXT`. ## Testing - `npm run typecheck -w @open-inspect/control-plane` - `npm test -w @open-inspect/control-plane` — 2313 passed - `npm run test:integration -w @open-inspect/control-plane` — 744 passed - `uv run --frozen pytest tests/` in `packages/modal-infra` — 176 passed (175 + the repaired one; 1 was failing on main before this PR) - `ruff check` / `ruff format --check` clean <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Breaking Changes** * Removed support for the legacy `build_timeout_seconds` field when creating image-build sandboxes. * Use `provider_session_timeout_seconds` to configure the provider session timeout. * **Bug Fixes** * Timeout validation, defaults, and limits now consistently use the supported timeout field. * **Tests** * Updated coverage to verify legacy fields are ignored and timeout settings remain independently supported. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary - derive `ArtifactType` and `SessionArtifact` from their canonical Zod schemas - remove the generic exported `recordSchema` helper from the artifact module - keep identical record validation local to the sandbox-event and media-request owners ## Scope This is an ownership-only refactor. It preserves the package-root API and serialized behavior, and does not add a package path, migrate consumers, add a facade, add tests, or add import enforcement. ## Architecture The artifact module retains artifact-specific schemas, including the already-supported `sessionArtifactSchema`. Generic record validation no longer creates a runtime dependency from sandbox events to artifacts. After this change, the artifact module has no unrelated exported helper blocking later direct-path adoption. ## Validation - `npm run build -w @open-inspect/shared` - `npm run typecheck -w @open-inspect/shared` - `npm test -w @open-inspect/shared` (39 files, 553 tests) - `npm test -w @open-inspect/shared -- src/module-boundaries.test.ts` (4 tests) - `npm run lint -w @open-inspect/shared` - `npm run format:check` - `npm run typecheck` (all workspaces) - root export and package export files unchanged - `git diff --check` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Improved internal validation of session artifacts and metadata. * Standardized artifact type definitions based on their validation schemas. * Preserved existing artifact validation behavior, including optional update timestamps. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary - move `spawnContextSchema` and `SpawnContext` out of `@open-inspect/shared` into the control-plane Session owner - migrate the Durable Object producer, route validator, and integration type reference to the internal module - relocate the existing schema tests with their owner ## Scope This is an ownership-only refactor. It preserves the schema fields and serialized behavior, removes the historical root exports that had no cross-package consumers, and does not add a package path, facade, new behavior, or import enforcement. ## Architecture The parent Durable Object spawn-context response contains encrypted SCM fields and is produced and consumed exclusively inside control-plane. `src/session/spawn-context.ts` now directly owns that internal response schema and inferred type. The broader shared Session HTTP API no longer exposes or depends on it. ## Validation - `npm run build -w @open-inspect/shared` - `npm run typecheck -w @open-inspect/shared` - `npm test -w @open-inspect/shared` (39 files, 547 tests) - `npm run lint -w @open-inspect/shared` - `npm run typecheck -w @open-inspect/control-plane` - `npm test -w @open-inspect/control-plane` (155 files, 2,319 tests) - `npm run test:integration -w @open-inspect/control-plane -- do-internal-routes.test.ts` (17 tests) - `npm run lint -w @open-inspect/control-plane` - `npm test -w @open-inspect/shared -- src/module-boundaries.test.ts` (4 tests) - `npm run format:check` - `npm run typecheck` (all workspaces) - `git diff --check`
…ray#1292) ## Summary A Slack mention whose repo-catalog fetch was slow **disappeared**. The `Working on *X*...` ack posted, and then nothing: no session, no error, no reply. The user re-mentioned the bot 15 minutes later and it worked, so it read as flakiness rather than a bug. The mention handler runs inside `waitUntil`, and `getAvailableRepos` called `GET /repos` with no bound. On a cold control-plane cache that request can take tens of seconds, at which point it has consumed the whole background-task budget and the platform cancels the remainder of the handler. The cancellation lands *after* the ack and *before* session creation, which is the worst possible window: - no session exists, so nothing can complete; - the ack is already posted, so the thread shows work in progress forever; - `startSessionAndSendPrompt`'s own failure branches — which do post "Sorry, I couldn't create a session" — never run. Production trace: ``` 12:07:08.123 app_mention received 12:07:32.950 control_plane.fetch_repos outcome=success repo_count=318 duration_ms=24390 12:07:37.315 (ack posted) 12:07:38.108 WARN waitUntil() tasks did not complete within the allowed time after invocation end and have been cancelled ``` The retry against a warm cache: `duration_ms=592`. ## Fix Bound the fetch at `REPOS_FETCH_TIMEOUT_MS` (5s). `getAvailableRepos` already falls back to the bot's own KV copy of the catalog on any error, so a timeout costs a possibly-stale catalog instead of the entire request. A warm fetch is well under a second, so the bound only trips when something is genuinely wrong. `controlPlaneFetch` gains an optional `timeoutMs`; callers that don't pass one are unchanged. ## Why a bound rather than making the fetch fast This pairs with ColeMurray#1273. That change made the control plane's cache refresh survive the *caller* disconnecting, which is exactly what now happens: the bot gives up at 5s, the refresh completes in the background, and the next mention is served from a warm cache. Before both changes an aborted miss left KV empty, so cold could persist indefinitely. ## Known gap, deliberately not in this PR The ack is still posted before the session exists, so any *other* way the handler dies leaves the same dangling "Working on..." message. Two ways to close that — post the ack only after session creation, or update it on failure — but the first trades away ~1s of feedback on the happy path and the second can't help when the isolate is killed outright. That's a UX call worth making separately from this root-cause fix; happy to follow up whichever way you prefer. ## Test plan - [x] `npm test -w @open-inspect/slack-bot` (407), `-w @open-inspect/shared` (545), `-w @open-inspect/control-plane` (2311) - [x] `npm run typecheck`, `npm run lint` clean - [x] New test in `src/classifier/repos.test.ts`: a fetch that rejects with `TimeoutError` returns the KV-cached catalog, and the request is asserted to carry an `AbortSignal` created with `REPOS_FETCH_TIMEOUT_MS`. It asserts the bound via a spy rather than waiting on real time, so it runs in single-digit milliseconds. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Repository catalog requests now time out after five seconds, preventing indefinitely stalled requests. * Existing cached repository data continues to be used when requests time out or fail. * **Tests** * Added coverage confirming timeout behavior and cached-data fallback. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Narrow hygiene pass over two control-plane background-work sites: 1. `durable-object.ts` no longer leaves `processMessageQueue()` as a floating promise when a sandbox WebSocket connects. The promise is registered with the existing Durable Object background-work pattern and unexpected failures are logged as `message_queue.process.background_error`. 2. `sandbox-events.ts` now attaches structured error handling to the post-completion snapshot task, logging unexpected failures as `snapshot.trigger.background_error` with the snapshot reason. ## Scope corrections - Child-spawn parent notification remains on the Worker `ExecutionContext.waitUntil()` path. It is a best-effort UI refresh, does not belong on the successful spawn response path, and later child status transitions also notify the parent. - `CallbackNotificationService.notifyComplete()` retains its existing contract: expected delivery failures are returned and logged, while unexpected internal failures are logged and rethrown. Promises passed to `waitUntil()` are observed by the runtime, so a service-wide swallow policy is unnecessary. - Durable Object `waitUntil()` is not described as extending object lifetime; Durable Objects remain active while they have ongoing work or pending I/O. The added catches provide explicit structured diagnostics. ## Behavior - No successful-path behavior changes. - Unexpected queue-processing and snapshot-task failures now produce structured logs. ## Tests - Added coverage that a rejected post-completion snapshot task is handled and logged. - The sandbox-connect flow remains covered by the control-plane integration suite; there is no direct SessionDO unit harness for the one-line queue-processing call site. ## Verification - Control-plane unit: **155 files, 2320 tests passed** - Control-plane integration (workerd + D1): **63 files, 744 tests passed** - Control-plane typecheck (source + tests): passed - Full repository lint and format check: passed
## Summary - expose the existing sandbox-event owner at `@open-inspect/shared/types/sandbox-events` - migrate control-plane, web, Slack, and Linear imports and reexports to that path - remove the sandbox-event symbols from both the shared root barrel and the control-plane type aggregator once all repository consumers have moved ## Scope This is a mechanical owner-path adoption. It does not change source declarations, schemas, serialized behavior, or runtime logic, and it does not add a facade, enforcement rule, or import-only tests. The repository now has zero imports or reexports of sandbox-event symbols from `@open-inspect/shared`. Control-plane consumers also import the sandbox-event owner directly instead of routing through `src/types.ts`. Shared-package tests use the existing relative owner path. ## Validation - `npm run typecheck` - `npm run format:check` - `npm test -w @open-inspect/shared` (547 tests) - `npm test -w @open-inspect/control-plane` (2,319 tests) - `npm test -w @open-inspect/web` (894 tests) - `npm test -w @open-inspect/slack-bot` (407 tests) - `npm test -w @open-inspect/linear-bot` (211 tests) - affected package production builds - `git diff --check`
## Summary - add a user-defined pull/merge-request label to provider-neutral SCM settings, including global defaults and repository overrides - resolve the effective SCM policy once for the canonical target repository before creating a pull request - ensure missing GitHub labels with read-before-create behavior, confirm `422` races instead of treating every validation failure as success, and preserve GitLab label mapping - add settings, resolution, multi-repository, GitHub, GitLab, and UI coverage ## Context This supersedes ColeMurray#867 because the original contributor fork cannot be updated. Thank you to @kadams54 for the original feature and UX direction. The replacement keeps session pull-request policy in the existing provider-neutral SCM settings model rather than GitHub Bot settings, and applies repository policy to the actual selected PR target in multi-repository sessions. ## Validation - `npm run lint` - `npm run typecheck` - `npm test -w @open-inspect/control-plane` (155 files, 2,328 tests) - `npm test -w @open-inspect/web` (115 files, 897 tests) - `npm run build -w @open-inspect/control-plane` - `npm run build -w @open-inspect/web` ## Review A thermo-nuclear maintainability review found one low-severity test-isolation issue in the new GitHub provider test. The warning spy is now restored in `finally`; the review found no other maintainability or scope issues. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added configurable labels for session-created pull and merge requests. * Labels can be set globally or customized per repository, with blank values inheriting global defaults. * Repository draft-mode settings can now be managed independently from labels and global defaults. * Pull request creation supports labels across GitHub and GitLab. * GitHub creates missing labels when possible without blocking pull request creation. * **Bug Fixes** * Improved handling of whitespace, invalid values, and label creation conflicts. * SCM settings failures now return a clear service-unavailable response. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
) ## Summary - calculate hard-slice fence repairs from the text retained in each Slack section rather than the final state of the full oversized token - guarantee repaired fenced sections remain within Slack's 3,000-character API limit - add a regression test for a long single-line fenced token that previously produced section lengths of 3,004 and 1,042 characters ## Root cause When one oversized token both opened and closed a code fence, the chunker inspected the fence state after the entire token and reserved no closing repair. The first hard slice could still end inside the fence, so balancing it added four characters after taking the full 3,000-character budget. ## Verification - `npm test -w @open-inspect/slack-bot` (408 tests) - `npm test -w @open-inspect/control-plane -- src/routes/slack-notify.test.ts` (21 tests) - `npm run lint -w @open-inspect/shared` - `npm run lint -w @open-inspect/slack-bot` - `npm run typecheck -w @open-inspect/shared` - `npm run typecheck -w @open-inspect/slack-bot` - `npm run build -w @open-inspect/shared` - `npm test -w @open-inspect/slack-bot -- src/completion/blocks.test.ts` (27 tests, rerun after commit hooks) --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/371c2771ee6babf58914bb22686d8a8f)* <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved handling of long Slack messages to keep sections within the 3,000-character limit. * Preserved balanced code blocks when splitting oversized content. * Improved Unicode text handling to prevent characters from being split incorrectly. * Improved preservation of whitespace, truncation markers, and code-block language information. * **Tests** * Added coverage for oversized tokens containing code fences. * Added comprehensive coverage for section limits, fence balancing, truncation, and edge cases. <!-- 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>
…tion (ColeMurray#1277) Fixes ColeMurray#1276. ## Problem Replying `owner/repo` to the bot's own repository clarification re-triggers the identical clarification forever — the documented flow ("That answer is used on the next resolution attempt", `docs/integrations/LINEAR.md`) never worked on the no-session path. ## Changes - **`webhook-handler.ts`** — a `prompted` event that reaches `handleNewSession` (i.e. no issue→session mapping exists) is a clarification reply; its text lives on `agentActivity.content.body`, so it now takes precedence over the session's original trigger comment when building the resolution context. - **`target-resolution.ts`** — new rung in the resolution ladder, ahead of Linear's suggestions API and LLM classification: `matchExplicitRepo` resolves deterministically when the comment names exactly one available repository. Case-insensitive (repos are stored lowercase), boundary-guarded (`acme/api` does not match inside `acme/api-legacy` or `notacme/api`), and multiple distinct matches still fall through to classification, since that is a genuine ambiguity. ## Testing - Unit tests for `matchExplicitRepo` (single/multi/none/boundary/case). - End-to-end handler test: `prompted` event with no session mapping + `acme/backend` reply → session created against `acme/backend`; the control-plane stub throws on any unexpected fetch, so the LLM classifier being consulted would fail the test. - `npm test -w @open-inspect/linear-bot`: 208/208. Typecheck + eslint clean. Observed in production: an issue describing UI work didn't match either repo description, and three consecutive exact `owner/repo` replies each re-ran cold classification with identical reasoning. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Explicit repository names in comments are now recognized with case-insensitive matching. * Repository resolution prioritizes clear repository references before heuristic classification. * Repository matching preserves the repository’s full name when resolved. * **Bug Fixes** * Improved clarification replies so explicitly specified repositories resolve correctly. * Prevented ambiguous, partial, or incorrectly delimited repository-name matches. * Ensured prompted activity is attributed to the responding actor when creating a session. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary - clarify that the existing GitHub App `Pull requests: Read & write` permission authorizes creating and applying labels to session-created pull requests - keep `Issues: Read & write` conditional on enabling the GitHub bot - align the canonical setup guide, onboarding workflow, web setup README, and GitHub bot permission guidance ## Context Commit `b63d017` added control-plane support for ensuring and applying labels to session-created GitHub pull requests. Although these operations use label endpoints under Issues API routes, GitHub App permission mapping authorizes them through `Pull requests: Read & write`; installations do not need an Issues permission upgrade solely for session PR labeling. Terraform configures GitHub App credentials but does not create or update the App itself, and there is no checked-in GitHub App permission manifest to change. ## Verification - `npx prettier --check "docs/GETTING_STARTED.md" "packages/web/README.md" ".claude/skills/onboarding/SKILL.md" "packages/github-bot/README.md"` - `git diff --check` - targeted searches confirmed no documentation still claims Issues permission is required for session PR labeling <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Clarified GitHub App permission requirements for contents, pull requests, issues, and metadata. - Documented that pull request write access supports labeling session-created pull requests. - Specified that Issues read/write access is required for labeling and GitHub bot functionality when enabled. - Added guidance for updating existing installations, including republishing settings and approving permission changes. <!-- 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>
Follow-up to ColeMurray#1277. ## Problem A second pass over the merged clarification-routing change found two contract edge cases: - Linear sends promptContext at the top level of an AgentSessionEvent payload, but the bot typed and read it from agentSession. Created sessions therefore discarded Linear's complete issue, thread, parent-issue, and guidance context and rebuilt a smaller fallback prompt. - Explicit repository matching rejected single-period continuations such as acme/backend.docs, but repeated periods such as acme/backend..docs could still produce a false acme/backend match. ## Changes - Model and consume promptContext at the webhook payload's top level. - Add a handler integration test that verifies the top-level context reaches the control-plane session prompt. - Scan the entire adjacent period run when deciding whether owner/repo is embedded in a longer path, while retaining ordinary terminal ellipses. - Verify that an unmapped prompted clarification reply retains its full instruction text in the newly created session prompt. - Document the actual five-stage repository-resolution cascade. ## Linear documentation - [Developing the Agent Interaction](https://linear.app/developers/agent-interaction) documents promptContext as the formatted context for created events and the separate prompted-event flow. - [Pinned AgentSessionEventWebhookPayload schema](https://github.com/linear/linear/blob/eabc85d0df87617b4647e56d2f236e60bc2ed117/packages/sdk/src/schema.graphql#L964-L1006) shows promptContext on the top-level event payload and marks it as present only for created events. ## Testing - npm run build -w @open-inspect/shared - npm test -w @open-inspect/linear-bot — 221 passed - npm run typecheck -w @open-inspect/linear-bot - npm run lint -w @open-inspect/linear-bot - npm run build -w @open-inspect/linear-bot - Prettier check and git diff --check An independent thermo-review found no actionable maintainability findings. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Improvements** - Repository detection now more accurately handles punctuation and path-like text, reducing incorrect matches. - Repository resolution documentation now explains the full five-step matching process. - Prompt context is consistently accepted and forwarded from webhook events, including clarification instructions. - **Bug Fixes** - Corrected handling of top-level prompt context for newly created sessions. - Added coverage for repository names followed by ellipses and rejection of ambiguous repeated-period matches. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Supersedes ColeMurray#1218 because the original source branch is not editable by the maintainers. The original implementation commit by @duboff is preserved with its authorship. - Fetch Slack thread history only after a channel-trigger invocation is admitted, and reuse one read across matching automations. - Keep Slack API access, display-name resolution, and thread rendering in `slack-bot`; the signed internal endpoint still returns `{ threadContext: string }`. - Select a bounded 20-message window with the thread root pinned, exclude the triggering and later replies, and allow text-bearing `file_share` messages to trigger. - Serialize untrusted thread messages as JSON with discriminated speaker identities so display names cannot impersonate the assistant or an app. ## Takeover fixes - Resolved the `packages/shared/src/slack/index.ts` conflict against current `main`, retaining both the current section exports and the thread-context exports. - Preserved speaker kind, Slack ID, and user display name as separate fields. - Made the triggering-message permalink an explicit typed Slack event field instead of a hidden dependency on the logging metadata bag. - Contained lazy prompt-provider failures inside `startInvocation`, falling back to the baseline prompt so admitted children cannot remain stranded in `starting`. - Enforced empty results for non-positive thread-window limits. - Avoided display-name lookups for app messages in the interactive thread path. - Hardened fetch-call and prompt-ordering assertions against false negatives. - Added direct `sinceTs`, non-positive-limit, typed-permalink, lazy-provider-failure, and adversarial display-name regressions. - Corrected docs to state that the root is included within the 20-message total. ## Design notes - The takeover does not introduce a shared thread renderer or a new typed RPC/auth framework. Slack continues to own thread rendering, and the endpoint preserves its existing string response. - Slack `conversations.replies` returns the earliest messages first and offers no reverse/tail mode. Preserving both the root and latest replies therefore requires pagination; the read is admission-bound, capped at 25 pages, and protected by the control-plane 10-second fallback timeout. - The configured bot token is the intended channel-history credential; repository setup already requires `channels:history` and `groups:history`. ## Validation - `npm test` (4,578 tests) - `npm run typecheck` - `npm run lint` - `npm run lint:complexity` - `npm run format:check` - `git diff --check origin/main...HEAD` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Slack automations can trigger on text-containing file-share messages; attachments are not forwarded, and image-only messages remain excluded. - Threaded runs can include up to 20 earlier messages with speaker labels, bounded content, and safe formatting. - Thread context loads only for admitted threaded runs and is shared across matching automations. - Conditions continue to evaluate the triggering message text only. - **Bug Fixes** - Slack history failures, timeouts, or unavailable data no longer prevent automation runs. - **Documentation** - Updated Slack automation guidance covering file-share handling and thread context behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Mikhail Dubov <mikhail@chattermill.io>
## Summary
- serve the canonical, secret-free session representation from `GET
/sessions/:id` for server rendering
- send the same `SessionSnapshot` shape on every WebSocket subscribe or
reconnect, then continue with the existing semantic live messages
- keep stable `eventId` and `timelineSequence` envelopes for
deterministic replay and history pagination
- fetch code-server and terminal credentials through the separate
authenticated, no-store `/sessions/:id/sandbox-access` endpoint
- keep rendered session content visible while reconnecting and gate live
actions until the socket snapshot is installed
The canonical representation is `{ session, artifacts, timeline,
spawnError }`. It has no duplicate session ID, bootstrap lifecycle
terminology, or parallel public state endpoint. Sandbox access is
explicitly limited to interactive sandbox services; integration and SCM
credentials retain their own domain-specific flows.
## Simplified architecture
1. The protected Next.js session route fetches and validates `GET
/sessions/:id` on the server.
2. A route-local provider gives the existing client page that snapshot
for its initial render.
3. The browser opens the existing session WebSocket.
4. After authentication and async enrichment, SessionDO performs a final
canonical SQLite snapshot read.
5. A synchronous handoff sends `subscribed`, registers the socket, and
persists its identity without an `await` between those operations.
6. A mutation is therefore either included in the snapshot or delivered
afterward as an ordered semantic WebSocket message.
7. Reconnect repeats the same bounded full-snapshot handoff; there is no
retained view-delta log or revision recovery state machine.
## Removed from the previous design
- `session_view_metadata` and `session_view_deltas` storage
- view revisions, per-socket applied revisions, and catch-up
byte/revision limits
- delta/snapshot/ready synchronization messages and reducer recovery
branches
- capability negotiation and legacy dual-protocol fan-out
- the duplicate `/sessions/:id/bootstrap` resource, duplicate
`sessionId`, and identity refinements
- the generic `/sessions/:id/access` name and `session_access_changed`
protocol event
- raw replay duplicates alongside stable timeline envelopes
- credential-bearing snapshots and WebSocket messages
- app-wide auth hydration and a duplicated 700+ line session client
- client-side timeline sorting, copied event envelopes, and redundant
integration scenarios
The simplification passes reduce the review surface from 4,147 changed
lines across 51 files to 1,975 lines across 41 files: 2,172 fewer
changed lines (52%).
## Correctness and security
- HTTP SSR and WebSocket subscribe share one schema and one SessionDO
snapshot builder
- the snapshot read, send, socket registration, and mapping persistence
form one synchronous handoff
- duplicate subscribe attempts are rejected, including sockets already
synchronizing
- malformed server messages enter bounded reconnect instead of leaving
the client connected but unready
- session snapshots never contain passwords or bearer tokens
- sandbox credentials are decrypted only for the authenticated
sandbox-access endpoint, with a post-decryption row recheck to reject
concurrent sandbox replacement
- sandbox credentials are cleared before reconnect/access invalidation
refetches and when sandbox lifecycle state makes them unusable
- stable event envelopes and cursors preserve ordered replay/history
pagination without a client revision state machine
## Verification
- shared, control-plane, and web TypeScript typechecks
- shared tests: 549 passed
- control-plane unit tests: 2,334 passed
- control-plane integration tests: 746 passed
- web tests: 911 passed
- shared and control-plane production builds; optimized Next.js
production build
- shared, control-plane, and web ESLint
- Prettier and `git diff --check`
The client and control-plane protocol changes are intentionally
coordinated; no legacy compatibility path remains.
---------
Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com>
## Summary - fetch one lookahead row when building the initial session replay - report `hasMore` only when that lookahead row exists - trim the oldest lookahead row so replay still returns at most 500 newest events - cover the exact 500-event and 501-event boundaries through the real WebSocket subscription flow ## Root cause The replay query fetched at most the configured limit and inferred additional history from `rows.length >= limit`. Exactly 500 events therefore looked indistinguishable from 501 or more events, causing an unnecessary empty history request. ## Testing - `npm run build -w @open-inspect/shared` - `npm test -w @open-inspect/control-plane` (2,344 passed) - `npm run test:integration -w @open-inspect/control-plane -- websocket-client.test.ts` (19 passed) - `npm run typecheck -w @open-inspect/control-plane` - scoped ESLint and Prettier checks - `git diff --check` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved event replay pagination and cursor handling. * Replay requests now return up to 500 events and accurately indicate when more are available. * Heartbeat events are excluded from replay results. * Improved handling of parsed and malformed replayed event data. * Added coverage for sessions containing exactly 500 and more than 500 events. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary - translate confirmed parent OpenCode `session.compacted` signals into a provider-neutral `context_compacted` sandbox event - persist, broadcast, filter, replay, and paginate compaction markers through the existing session event pipeline - render a neutral `Context compacted to continue` row in the session timeline without exposing internal summaries - document scope, compatibility, rollout guidance, risks, and acceptance criteria - add TDD coverage across shared schemas, sandbox runtime, control plane unit/integration paths, and web live/replay/history rendering ## Scope This first release intentionally covers successful parent-session compaction only. Child Task-session compactions, inferred token/context metrics, summary exposure, and ACK-aware exact delivery remain out of scope. ## Verification - `npm run typecheck` - `npm test -w @open-inspect/shared` (578 tests) - `npm test -w @open-inspect/control-plane` (2341 tests) - `npm test -w @open-inspect/web` (902 tests) - `npm run test:integration -w @open-inspect/control-plane` (746 tests) - `NODE_ENV=production npm run build -w @open-inspect/web` - touched-package ESLint for shared, control-plane, and web - focused sandbox runtime compaction suites (87 tests) - sandbox runtime Ruff checks and formatting - mypy for `prompt_stream.py` The clean-environment full sandbox runtime suite passed 715 of 716 tests; the remaining unrelated signer integration assertion depends on the installed Git version's SSH signing invocation (`-U`). Root lint also reports pre-existing generated `.opencode` global errors, while all touched package lint checks pass. --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/a0bb835bd01b9dfecf5c397b5c063ed6)* <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for displaying “Context compacted” markers in session timelines. * Context compaction events are now preserved in session history and replayed in chronological order. * Live updates broadcast compaction markers without disrupting ongoing assistant text or session activity. * Repeated parent-session compactions are recorded individually, while child-session compactions remain isolated. * **Bug Fixes** * Event validation now consistently accepts supported event types and filters unrelated events from event history. <!-- end of auto-generated comment: release notes by coderabbit.ai --> <img width="390" height="844" alt="image" src="https://github.com/user-attachments/assets/8ab52663-b50d-487b-b652-a64b9bc330d5" /> --------- Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com>
…leMurray#1316) Bumps the uv group with 1 update in the /packages/e2b-infra directory: [h2](https://github.com/python-hyper/h2). Bumps the uv group with 1 update in the /packages/modal-infra directory: [h2](https://github.com/python-hyper/h2). Updates `h2` from 4.3.0 to 4.4.1 <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/python-hyper/h2/blob/master/CHANGELOG.rst">h2's changelog</a>.</em></p> <blockquote> <h2>4.4.1 (2026-08-03)</h2> <p><strong>Bugfixes</strong></p> <ul> <li>Performance improvement: remove consumed frames in-place from data buffer.</li> <li>Reject duplicate Host headers in request headers. Thanks to Sunand Mohan for the report.</li> </ul> <h2>4.4.0 (2026-07-23)</h2> <p><strong>API Changes (Backward Incompatible)</strong></p> <ul> <li>Support for Python 3.9 has been removed.</li> <li>Support for PyPy 3.9 has been removed.</li> <li><code>Stream.end_stream()</code> now raises <code>NoSuchStreamError</code> or <code>StreamClosedError</code> exceptions, instead of a generic <code>KeyError</code>.</li> <li>Duplicate <code>content-length</code> headers with different values now raise <code>ProtocolError</code>. Previously, the first <code>content-length</code> header was accepted and later conflicting values were ignored. Thanks to Harshal Parekh for the report.</li> <li>Parse <code>content-length</code> headers according to RFC9110 grammar for numbers (1*DIGIT). Thanks to Arkadiusz Marta for the report.</li> <li><strong>backfill from v4.3.0</strong> Convert emitted events into Python <code>dataclass</code>, which introduces new constructors with required arguments. Instantiating these events without arguments, as previously commonly used API pattern, will no longer work.</li> </ul> <p><strong>API Changes (Backward Compatible)</strong></p> <ul> <li>Support for Python 3.14 has been added.</li> <li><code>H2Connection.receive_data</code> now accepts any byte-like object that implements the buffer protocol, such as <code>bytes</code>, <code>bytearray</code>, and <code>memoryview</code>. Existing <code>bytes</code> callers are unaffected.</li> <li>Align CONNECT pseudo-header validation with RFC 9113 s8.3 and RFC 8441 s4. Ordinary CONNECT now requires <code>:method=CONNECT</code> and <code>:authority</code>, and forbids <code>:scheme</code>/<code>:path</code>. Extended CONNECT (e.g., WebSocket) requires <code>:scheme</code>, <code>:path</code>, <code>:authority</code> plus <code>:protocol</code>. (PR <a href="https://redirect.github.com/python-hyper/h2/issues/1309">#1309</a>)</li> <li>Fix incorrect substring matching of secure header in <code>cookie</code> and <code>:method</code>.</li> </ul> <p><strong>Bugfixes</strong></p> <ul> <li>Fix to allow sending 0 bytes on a stream even if the flow control window is negative.</li> <li>Reject non-zero <code>SETTINGS_ENABLE_PUSH</code> values received from servers.</li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/python-hyper/h2/commit/bc239af1d1b85bc70482804f30a0e0e587d90a08"><code>bc239af</code></a> v4.4.1</li> <li><a href="https://github.com/python-hyper/h2/commit/92b925ed1b1817c82db32893503f74f47fcf4452"><code>92b925e</code></a> add test for duplicate host headers</li> <li><a href="https://github.com/python-hyper/h2/commit/292a40829feefda98c8509dcdbbb4a57af9bd6a6"><code>292a408</code></a> reject duplicate Host headers in request headers</li> <li><a href="https://github.com/python-hyper/h2/commit/04d3b87cbc1db020d28c7cfb44fe194558efbdde"><code>04d3b87</code></a> update changelog</li> <li><a href="https://github.com/python-hyper/h2/commit/439b970d0fa19891fa81068907de97dfa3a07c3a"><code>439b970</code></a> prepare for next release cycle</li> <li><a href="https://github.com/python-hyper/h2/commit/9a7ff7430df669fa8e90b6121f3cc1ed64d1115a"><code>9a7ff74</code></a> performance: remove consumed frames in place from data buffer (<a href="https://redirect.github.com/python-hyper/h2/issues/1321">#1321</a>)</li> <li><a href="https://github.com/python-hyper/h2/commit/6cce763997eca5b826f3e435a611b7a7fc73f633"><code>6cce763</code></a> v4.4.0</li> <li><a href="https://github.com/python-hyper/h2/commit/dfafda3b0cd96455b45d1785ef1ebc6968bba5cf"><code>dfafda3</code></a> Bump pytest from 8.4.2 to 9.0.3 (<a href="https://redirect.github.com/python-hyper/h2/issues/1320">#1320</a>)</li> <li><a href="https://github.com/python-hyper/h2/commit/b45207cedf9fabe2c77bb3c1c10f403a610599a0"><code>b45207c</code></a> dependencies and packaging++</li> <li><a href="https://github.com/python-hyper/h2/commit/c40145f69c5473850849fe96301ac0416b1afea6"><code>c40145f</code></a> parse <code>content-length</code> headers according to RFC9110 grammar for numbers (1*DI...</li> <li>Additional commits viewable in <a href="https://github.com/python-hyper/h2/compare/v4.3.0...v4.4.1">compare view</a></li> </ul> </details> <br /> Updates `h2` from 4.3.0 to 4.4.1 <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/python-hyper/h2/blob/master/CHANGELOG.rst">h2's changelog</a>.</em></p> <blockquote> <h2>4.4.1 (2026-08-03)</h2> <p><strong>Bugfixes</strong></p> <ul> <li>Performance improvement: remove consumed frames in-place from data buffer.</li> <li>Reject duplicate Host headers in request headers. Thanks to Sunand Mohan for the report.</li> </ul> <h2>4.4.0 (2026-07-23)</h2> <p><strong>API Changes (Backward Incompatible)</strong></p> <ul> <li>Support for Python 3.9 has been removed.</li> <li>Support for PyPy 3.9 has been removed.</li> <li><code>Stream.end_stream()</code> now raises <code>NoSuchStreamError</code> or <code>StreamClosedError</code> exceptions, instead of a generic <code>KeyError</code>.</li> <li>Duplicate <code>content-length</code> headers with different values now raise <code>ProtocolError</code>. Previously, the first <code>content-length</code> header was accepted and later conflicting values were ignored. Thanks to Harshal Parekh for the report.</li> <li>Parse <code>content-length</code> headers according to RFC9110 grammar for numbers (1*DIGIT). Thanks to Arkadiusz Marta for the report.</li> <li><strong>backfill from v4.3.0</strong> Convert emitted events into Python <code>dataclass</code>, which introduces new constructors with required arguments. Instantiating these events without arguments, as previously commonly used API pattern, will no longer work.</li> </ul> <p><strong>API Changes (Backward Compatible)</strong></p> <ul> <li>Support for Python 3.14 has been added.</li> <li><code>H2Connection.receive_data</code> now accepts any byte-like object that implements the buffer protocol, such as <code>bytes</code>, <code>bytearray</code>, and <code>memoryview</code>. Existing <code>bytes</code> callers are unaffected.</li> <li>Align CONNECT pseudo-header validation with RFC 9113 s8.3 and RFC 8441 s4. Ordinary CONNECT now requires <code>:method=CONNECT</code> and <code>:authority</code>, and forbids <code>:scheme</code>/<code>:path</code>. Extended CONNECT (e.g., WebSocket) requires <code>:scheme</code>, <code>:path</code>, <code>:authority</code> plus <code>:protocol</code>. (PR <a href="https://redirect.github.com/python-hyper/h2/issues/1309">#1309</a>)</li> <li>Fix incorrect substring matching of secure header in <code>cookie</code> and <code>:method</code>.</li> </ul> <p><strong>Bugfixes</strong></p> <ul> <li>Fix to allow sending 0 bytes on a stream even if the flow control window is negative.</li> <li>Reject non-zero <code>SETTINGS_ENABLE_PUSH</code> values received from servers.</li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/python-hyper/h2/commit/bc239af1d1b85bc70482804f30a0e0e587d90a08"><code>bc239af</code></a> v4.4.1</li> <li><a href="https://github.com/python-hyper/h2/commit/92b925ed1b1817c82db32893503f74f47fcf4452"><code>92b925e</code></a> add test for duplicate host headers</li> <li><a href="https://github.com/python-hyper/h2/commit/292a40829feefda98c8509dcdbbb4a57af9bd6a6"><code>292a408</code></a> reject duplicate Host headers in request headers</li> <li><a href="https://github.com/python-hyper/h2/commit/04d3b87cbc1db020d28c7cfb44fe194558efbdde"><code>04d3b87</code></a> update changelog</li> <li><a href="https://github.com/python-hyper/h2/commit/439b970d0fa19891fa81068907de97dfa3a07c3a"><code>439b970</code></a> prepare for next release cycle</li> <li><a href="https://github.com/python-hyper/h2/commit/9a7ff7430df669fa8e90b6121f3cc1ed64d1115a"><code>9a7ff74</code></a> performance: remove consumed frames in place from data buffer (<a href="https://redirect.github.com/python-hyper/h2/issues/1321">#1321</a>)</li> <li><a href="https://github.com/python-hyper/h2/commit/6cce763997eca5b826f3e435a611b7a7fc73f633"><code>6cce763</code></a> v4.4.0</li> <li><a href="https://github.com/python-hyper/h2/commit/dfafda3b0cd96455b45d1785ef1ebc6968bba5cf"><code>dfafda3</code></a> Bump pytest from 8.4.2 to 9.0.3 (<a href="https://redirect.github.com/python-hyper/h2/issues/1320">#1320</a>)</li> <li><a href="https://github.com/python-hyper/h2/commit/b45207cedf9fabe2c77bb3c1c10f403a610599a0"><code>b45207c</code></a> dependencies and packaging++</li> <li><a href="https://github.com/python-hyper/h2/commit/c40145f69c5473850849fe96301ac0416b1afea6"><code>c40145f</code></a> parse <code>content-length</code> headers according to RFC9110 grammar for numbers (1*DI...</li> <li>Additional commits viewable in <a href="https://github.com/python-hyper/h2/compare/v4.3.0...v4.4.1">compare view</a></li> </ul> </details> <br /> 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>
## Summary - add the sandbox-only `send-child-prompt` tool for queueing follow-up instructions in direct child sessions - add a parent-scoped control-plane route with D1 and child Durable Object lineage checks - reuse the child session's durable prompt queue, owner attribution, sandbox restoration, and event pipeline - support completed/failed child resumption while keeping cancelled/archived sessions terminal - add prompt size, queue depth, and best-effort concurrent-child admission guardrails - harden cancellation, archival, prompt dispatch, deterministic FIFO ordering, and late completion handling - expose unfinished child work so prior results are labeled accurately - add the implementation plan and HTML architecture presentation ## Security and lifecycle - validates the parent sandbox token against the parent session path - restricts follow-ups to direct children and verifies lineage again in the child Durable Object - derives author identity from the child owner; callers cannot provide source, identity, callbacks, attachments, or model overrides - terminalizes processing and pending prompts with completion events and callbacks during cancellation - prevents late sandbox completion events from overwriting synthetic cancellation results ## Reviews - completed an independent implementation-to-specification audit and corrected all actionable deviations - completed two independent self-review passes and iterated on every finding - retained one improvement over the original specification: transport execution is isolated in a testable helper, and tool naming follows the newer explicit child-session convention (`send-child-prompt`) ## Verification - `npm test -w @open-inspect/shared` (528 tests) - `npm test -w @open-inspect/control-plane` (2,219 tests) - `npm run test:integration -w @open-inspect/control-plane` (687 tests) - `npm run typecheck` - runtime Node tests (15 tests) - sandbox tool installation tests (25 tests) - Ruff checks - Prettier and `git diff --check` The repository-wide Python suite was also run, but it is not hermetic inside a live restored Open-Inspect sandbox: 24 unrelated tests inherit active restore/auth/OAuth environment variables. The changed Python installation test and all runtime tests relevant to this feature pass. --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/e09c806b5fad413383a707625617cb66)* <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added the `send-child-prompt` tool for queueing follow-up instructions in direct child sessions. * Added validation, ownership checks, queue limits, and clear error reporting. * Child status now indicates when a follow-up prompt is queued or processing. * Improved enforcement of concurrent child-session limits. * **Bug Fixes** * Improved handling of cancelled, archived, and completed sessions. * Ensured queued work is cancelled and unfinished messages are finalized consistently. * **Documentation** * Documented child-session follow-up prompts, lifecycle behavior, authentication, and status retrieval. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com>
## Summary - add opt-in browser-based VNC desktops across the sandbox runtime, providers, control plane, and web session UI - provision authenticated noVNC access and lifecycle-managed credentials - preserve the generic v56 spawn compatibility floor while the background rebuild policy converges prebuilt images to the VNC-capable v57 runtime ## Review iteration This supersedes ColeMurray#1293 without modifying the contributor branch. It retains the original commits and authorship from @leejayhsu, includes the current `main` conflict resolution, and applies the follow-up requested during review. VNC startup remains best-effort. During the rebuild gap, a VNC-enabled session may select a compatible v56 image: the core sandbox and agent still boot, while the VNC endpoint remains unavailable until a new prebuilt image is ready. New sessions use the rebuilt image; a session created during the gap may retain its older runtime snapshot. ## Validation - `npm run build -w @open-inspect/shared` - `npm test -w @open-inspect/control-plane -- src/image-builds/rebuild-policy.test.ts src/sandbox/lifecycle/image-selection.test.ts src/sandbox/lifecycle/manager.test.ts` (113 tests) - `npm run test:integration -w @open-inspect/control-plane -- test/integration/image-builds.test.ts` (50 tests) - `npm run typecheck -w @open-inspect/control-plane` - targeted ESLint and Prettier checks for all changed control-plane files <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added optional VNC Desktop access for sandbox sessions, including noVNC links and connection credentials. * Added VNC enablement controls at global, environment, and repository levels. * Added configurable VNC service-port settings with conflict validation. * VNC access now works across sandbox creation, restoration, and resumption. * Added VNC status and access links to the session sidebar. * **Security** * Sensitive VNC passwords are protected and excluded from snapshots and real-time messages. * **Bug Fixes** * Improved cleanup of VNC access data when sandboxes stop or restart. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Leejay Hsu <37034741+leejayhsu@users.noreply.github.com>
## Summary - add an August 9 changelog section covering browser-based sandbox desktops - document pull request draft and label policies, child-session follow-ups, and per-user unread outcomes - highlight Slack thread context and the more resilient session timeline experience ## Validation - `npx prettier --check CHANGELOG.md` - `git diff --check -- CHANGELOG.md` --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/a923c14525b7fd15666b33cb51b8c5ca)* <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added browser-based sandbox desktops for isolated work. - Added follow-up prompts for child sessions. - Improved context-aware Slack automations. - Made session timelines more resilient. - Added pull-request labels and clearer draft policies. - Added unread indicators for session outcomes. - **Documentation** - Added changelog entries for August 3, 5, 7, 8, and 9, 2026. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com>
## Summary - refetch code-server and VNC access when the sandbox reaches `ready` or `running` - add regression coverage for the access-change/readiness event sequence ## Testing - `npm test -w @open-inspect/web -- src/hooks/use-session-socket.test.tsx src/hooks/use-sandbox-access.test.tsx` - `npm run typecheck -w @open-inspect/web` - `npx eslint src/hooks/use-session-socket.ts src/hooks/use-session-socket.test.tsx` --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/e233f0be26b9430cbef24511c3ea2f50)* <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Sandbox access updates are now announced only after the sandbox is fully ready. * Improved event ordering so readiness status appears before access details. * Prevented premature access notifications during sandbox startup, restoration, or resume. * Confirmed sandbox access remains available once the connection is successfully established. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com>
## Problem `main` currently has two migrations numbered `0067`: ``` terraform/d1/migrations/0067_default_single_provider_accounts.sql # ColeMurray#1561 terraform/d1/migrations/0067_keyboard_shortcut_preferences.sql # ColeMurray#1562 ``` `scripts/d1-migrate.sh` validates prefixes up front and **exits non-zero** on a duplicate: ``` ERROR: duplicate migration version prefixes detected: 0067 Renumber the colliding files so each prefix is unique before deploying. ``` Because that runs inside `terraform apply`, this is not a contained failure — the whole apply fails, so the workers and the data plane don't ship either. The script's own comment anticipates exactly this case ("two PRs that each grab the next number and then both merge"). There's a second, quieter problem. `migration-0067-default-single-provider-accounts.test.ts` resolves its migration by numeric prefix: ```ts const migration = env.TEST_MIGRATIONS.find((entry) => entry.name.startsWith("0067")); ``` With two `0067` files, `find` returns whichever the loader yields first, so the test can apply `keyboard_shortcut_preferences` and then assert against `model_provider_account_defaults`. ## Fix ColeMurray#1562 merged first (2026-08-21 23:41) and keeps `0067`; ColeMurray#1561's migration (2026-08-22 22:03) moves to `0068`. The two are independent — one creates `keyboard_shortcut_preferences`, the other backfills `model_provider_account_defaults` — so relative order doesn't matter. The test is renamed and repointed at `0068`. Both files are renames; the SQL content is untouched. ## Verification The equivalent change is already deployed downstream, where the ledger applied both cleanly under distinct versions: | version | name | |---|---| | `0068` | `0068_default_single_provider_accounts.sql` | | `0067` | `0067_keyboard_shortcut_preferences.sql` | `0068`'s backfill is a safe no-op where a provider already has a default (its `NOT EXISTS` guard), and the integration test binds to the right migration and passes. ## Note for anyone already running `main` If an installation applied these before this fix, its `_schema_migrations` already records `0067` under one of the two names. `d1-migrate.sh` errors when a version is recorded under a different name, so those installations should confirm their ledger matches this numbering (`0067` = `keyboard_shortcut_preferences`) before the next deploy. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Automatically creates default provider-account records when an active provider has exactly one eligible account and no default is configured. * Preserves account metadata and sets the appropriate unattended mode. * **Bug Fixes** * Corrected migration validation to reference migration 0068 and report the matching missing-migration error. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…y#1568) ## Summary - replace separate model and reasoning-effort controls with one consolidated selector - use nested model and effort menus on desktop and an in-place drill-down flow on mobile - cap long model lists to four visible rows with scrolling - align desktop submenus with the bottom of the session input while preserving viewport collision handling - apply the shared selector to both new-session and session-detail prompt composers - remove the old cycling effort control and add focused interaction coverage ## Verification - `npm test` in `packages/web` (1,197 tests passed) - `npm run typecheck` in `packages/web` - `npm run lint -- --no-warn-ignored` in `packages/web` ## Visual verification - desktop model menu alignment: artifact `220af008827218befb8834d284a0e46c` - desktop effort menu alignment: artifact `a411eea87eed78dc6063a0f97d5f0f8c` - mobile drill-in behavior: artifact `18074e4e714c855069dbd1d2cc868c78` --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/003664f10a683c8debf41f80cdf7d70f)* <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a unified **Model and effort** selector for choosing models and reasoning levels. * Added responsive menus with nested navigation on desktop and mobile. * Added grouped model options and clearly formatted reasoning-effort choices. * The selector reflects current selections and supports disabled states. * **Improvements** * Replaced separate model and reasoning controls with one consistent interface across the app. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com>
## Summary - display `now` instead of `just now` for timestamps less than one minute old - keep provider account relative-date labels grammatically correct - add regression coverage for the compact timestamp ## Testing - `npm test -w @open-inspect/web -- src/lib/time.test.ts` - `npm run lint -w @open-inspect/web` - `npm run typecheck -w @open-inspect/web` --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/8a028e050e0355e21bbc0b3fa982cc15)* <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Updated relative timestamps from “just now” to the more concise “now” for events occurring within the past minute. * Provider account settings now consistently display the updated relative time label. * **Tests** * Added coverage confirming timestamps from 30 seconds ago display as “now.” <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com>
## Summary - move repository and branch selectors above the new-session composer - reduce provider authentication to a trailing kebab menu after skills - remove decorative icons from repository, branch, and model selectors - preserve accessible authentication selection labels and existing menu behavior ## Testing - `npm test -w @open-inspect/web -- --run src/components/model-reasoning-selector.test.tsx src/components/session-prompt-composer.test.tsx 'src/app/(app)/page.test.tsx'` - `npm run typecheck -w @open-inspect/web` - Prettier and `git diff --check` ## Visual verification - Desktop viewport, 1440x900: artifact `362591d2fcbb9424a10589b8810821c8` - Mobile viewport with collapsed sidebar, 390x844: artifact `0a25ce8dec63a49935c0b17d16ed5da8` --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/829d4fddb8b2482f43174a5cf858178c)* <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **UI Improvements** - Moved repository and branch selectors above the prompt composer for easier access. - Grouped skill, model, and provider options into the session controls area. - Simplified provider and model controls with cleaner, icon-focused triggers while preserving accessible labels. - Updated repository and branch menus to open downward. - Removed redundant leading icons from repository and branch selectors. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com>
Replace the visible "OpenAI" / "xAI" labels in provider authentication controls with the existing logos. The compact composer trigger now shows the logo plus the selected account name. The session-options submenu and automations form labels use the same logos. Accessible names and titles still include the provider name. --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/f881c9167aa50bbaef1d2f73837eccde)* <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Provider authentication controls now display the relevant OpenAI or Grok logo. - Added accessible labels to authentication menus and selectors for improved usability with assistive technologies. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com>
## Summary - document user-facing features and notable improvements merged from August 16 through August 22 - add entries for provider accounts, session inbox organization, managed-skill improvements, E2B prebuilt images, keyboard shortcuts, automation activity, sandbox diagnostics, and model selection - move the OpenCode Zen GLM 5.2 entry to its actual August 21 merge date ## Validation - `npm exec prettier -- --check CHANGELOG.md` - `git diff --check` --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/b0e92dcd6a322300f6b85199a95390ec)* <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added unified model and reasoning selection. * Added configurable keyboard shortcuts and automation activity visibility. * Added managed provider accounts and managed-skill autocomplete. * Added E2B image support, sandbox snapshots, and session attention indicators. * **Bug Fixes** * Improved sandbox failure handling and clarified snapshot compatibility behavior. * **Documentation** * Updated release notes for August 16, 20, 21, and 22, 2026. * Clarified provider account availability and sandbox error conditions. * Removed the OpenCode Zen GLM 5.2 entry. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com>
…ColeMurray#1577) ## What Ports a guard that production already carries but public did not: a sandbox WebSocket upgrade against a session that is closed for good is rejected with `410 Session is terminal`. - `sandbox/lifecycle/decisions.ts` — adds `SANDBOX_RECONNECT_BLOCKED_SESSION_STATUSES` and the `isSessionSandboxReconnectBlocked` predicate. - `session/durable-object.ts` — adds the guard in `handleWebSocketUpgrade`, immediately after sandbox token validation. **Blocked membership is exactly `{archived, cancelled}`.** `created`, `active`, `completed` and `failed` are all allowed through. That split is the part worth reading twice. `completed` and `failed` read as terminal but are not — a completed session is idle, not over. Warm-on-typing spawns a sandbox for one *before* the follow-up prompt arrives, so rejecting its bridge returned 410 to every reconnect and stranded the follow-up. Production hit exactly that bug and removed the two statuses from the set. This PR reproduces the post-fix membership verbatim rather than re-deriving it; I confirmed no later production commit re-tuned the set. ## Why Public and prod had diverged here, and this file is about to be touched by the SessionDO decomposition. **This is Wave A, item 2 of 3 of that plan** — the point of Wave A is to land the small behavioural deltas first so the decomposition itself can be a pure structural move rather than a refactor that also silently changes admission behaviour. ## Guard placement (differs from the original port) The guard sits **after** `isValidSandboxToken`, not before it. The first draft of this PR placed it before, and review caught a real defect: `isValidSandboxToken` awaits `crypto.subtle.digest`. That is a *non-storage* await, so the Durable Object input gate does not hold other events back while it runs. A cancel or archive landing mid-hash left the pre-await status read stale, and the upgrade was accepted (`101`) — attaching a live bridge to a closed session and flipping the sandbox back to `ready` via `updateSandboxStatus("ready")`. This was demonstrated, not theorised: the regression test below failed with `expected 101 to be 410` before the guard was moved. Reading after authentication also avoids widening an unauthenticated status oracle. `/sessions/:id/ws` routes straight to the DO with no worker-level auth, so a pre-auth guard would have let an unauthenticated caller holding only a session ID distinguish archived/cancelled (`410`) from active (`403`/`401`). ## Testing - Unit: `isSessionSandboxReconnectBlocked` covered for all six `SessionStatus` values, with the blocked/allowed split declared explicitly per status rather than computed from the set the implementation uses. - Integration: archived and cancelled upgrades get `410`; completed and failed upgrades still connect and reach `ready`; and a mid-auth cancellation is re-checked after the await, asserting both the `410` and that the sandbox stays `stopped`. - Mutation-checked rather than just eyeballed. Adding `completed`/`failed` back to the blocked set fails 4 tests (the exact user-visible regression described above). Disabling the guard fails 3, including the mid-auth race. The archived/cancelled cases seed the sandbox as `ready`, so the pre-existing sandbox-status guard cannot produce a false-positive `410`. - Full suites: 3042 unit tests and 965 integration tests pass; `typecheck` clean across both tsconfigs; prettier and eslint clean. ## Deliberately not in scope Both `decisions.ts` and `decisions.test.ts` are now byte-identical to production, so they merge clean on the next sync. `durable-object.ts` is **not** fully equivalent, and two pre-existing public gaps are left alone here: 1. Public's sandbox-status guard still runs pre-auth off a stale read; prod re-reads it post-auth. 2. Public has no equivalent of prod's `Forbidden: Sandbox credentials changed` check. Both come from prod's `28fe6416`, which also touches `session-status-service.ts`, `websocket-manager.ts` and a conformance suite. Folding that in would have made this PR a behaviour change to existing authenticated paths rather than an additive port. One user-visible consequence of leaving it: when a session is terminal *and* the sandbox is already stopped at request time, public returns `410 Sandbox is stopped` where prod returns `410 Session is terminal` — same status code, different body. Worth closing separately as part of the decomposition. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Prevented prompts, model selection, and sandbox connections for archived or cancelled sessions. - Improved handling of session status changes during sandbox authentication. - Completed and failed sessions can now connect and transition to ready state correctly. - **Tests** - Added coverage for terminal session states, authentication races, and rejected sandbox connections. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com>
…ycle (ColeMurray#1578) ## What `SessionMessageQueue` and `SandboxLifecycleManager` were mutually un-constructible: the queue takes the manager as a constructor argument, and the manager took the queue's two termination callbacks as a constructor argument. `SessionDO` only got away with it because both getters are lazy and the callback literals deferred the `this.messageQueue` lookup until fire time. This PR moves the callbacks out of the manager's constructor and behind a new `setCallbacks(callbacks: LifecycleCallbacks): void`: - `manager.ts` — `callbacks` goes from a `readonly` constructor parameter to a private field wired once by `setCallbacks`. All 7 call sites still invoke them optionally (`?.()`), so an unwired manager is fully functional and just skips the notification, exactly as before. - `durable-object.ts` — the `lifecycleManager` getter constructs the manager and then wires it. `createLifecycleManager()` no longer references `messageQueue` at all. `setCallbacks` is deliberately **guarded, not idempotent**: a second call throws. This is one-time wiring by whoever owns the manager's construction; a second caller means two owners disagree about which collaborator gets notified, and letting the last one win would silently drop terminations. Documented on the method. ## Why Part of the SessionDO decomposition plan — **Wave A, item 3 of 3**. This was the only construction cycle in the SessionDO object graph. Removing it is what lets a later phase replace the lazy getters with a composition root that builds the graph eagerly. To be precise about what is and is not done here: the callbacks still resolve `this.messageQueue` lazily at fire time, so the DO has not yet stopped depending on lazy construction. What changed is that the cycle is no longer forced by the constructor signatures — an eager composition root can now pass a concrete `messageQueue` reference, which was impossible before. The DAG property is **enabled, not yet exercised**; Phase 2 removes the getters and does the rest. ## Behaviour Zero functional change. The two closures are textually identical to the ones they replace and are wired before the manager is reachable by anything, so the callbacks fire in exactly the same situations. One signature note: `imageBuildLookup` shifts from constructor arg 9 to arg 8. That is safe by construction — `ImageBuildLookup` has two required members, so any caller still passing a `LifecycleCallbacks` literal in that slot fails typecheck rather than binding silently. ## Testing - `packages/control-plane/src/sandbox/lifecycle/manager.test.ts` — new `setCallbacks` block: callbacks fire on the heartbeat-stale and `terminateUnresponsiveSandbox` paths after post-construction wiring; an unwired manager does not throw on a terminated path; a second wiring throws and leaves the first wiring intact. Existing tests that passed callbacks positionally were rewritten to `manager.setCallbacks(...)` — none deleted. - `packages/control-plane/test/integration/session-lifecycle-callbacks.test.ts` — new. Drives a real `SessionDO`: parks its sandbox past the connecting timeout with a `processing` message outstanding, runs the alarm, and asserts the message is failed. Plus one test that the DO wires the manager exactly once. Added because review found the DO-side wiring had no coverage at all — deleting the `setCallbacks` call left the entire unit suite green, and the production failure mode is silent (a `processing` message stuck forever, no error anywhere). Both new tests were mutation-verified: they fail when the wiring call is removed. - `npm test -w @open-inspect/control-plane` — 195 files / 3041 tests pass. - `npm run typecheck -w @open-inspect/control-plane` — clean. - `npm run test:integration -w @open-inspect/control-plane -- test/integration/session-lifecycle-callbacks.test.ts` — 2 tests pass. - prettier + eslint clean on all changed files. ## Not done / deferred - The full integration suite was not run locally, only the new file. Nothing here changes a store or D1 signature, and no pre-existing integration test references the callback names. CI shards the suite. - The lazy getters in `durable-object.ts` stay, by design — Phase 2 removes them. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved recovery when sandbox operations fail, time out, or become unresponsive. * Stuck processing messages are now marked as failed during lifecycle recovery. * Queued prompts automatically resume after sandbox termination. * Stop-confirmation state is cleared during recovery. * **Tests** * Added integration coverage for connecting-timeout recovery. * Expanded coverage for sandbox failures, termination outcomes, and queue resumption. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com>
…ColeMurray#1579) ## What Test-only change. Adds four integration tests covering the **eviction / hibernation restore path** of `SessionDO`, in the existing `test/integration/durable-object.test.ts` under a new `eviction and hibernation restore` describe block: 1. **Client prompt on a reconstructed instance** — a client frame routed on an instance that has not run `ensureInitialized()` in memory is still handled, and the prompt is persisted against the right participant. 2. **Alarm on a reconstructed instance** — an alarm delivered to a reconstructed instance runs and its stuck-message watchdog fails the message. 3. **Client identity recovered from storage** — with the in-memory `ClientInfo` cache gone, a socket whose row survives in `ws_client_mapping` is recovered, and every rebuilt identity field (participant id, canonical user id, name, avatar) is asserted through the resulting `presence_update` broadcast. 4. **Missing mapping closes the socket** — the negative case: no mapping row gives close code 4002. Eviction is forced with `ctx.abort()`, which discards every in-memory field while leaving the DO's SQLite intact. A test-only marker is stamped onto the instance before the abort and asserted absent afterwards, so the tests cannot silently stop exercising reconstruction if the harness ever makes `abort()` a no-op. The only other change is in `test/integration/helpers.ts`: `openClientWs` gains optional `scmLogin` and `scmName` passthroughs. These are additive optionals, so `JSON.stringify` drops them and the `/internal/ws-token` request body is byte-identical for all 45 existing call sites. **No `src/` changes.** `git diff origin/main...HEAD --name-only` is exactly those two test files. ## Why Wave A, item 1 of 3 of the SessionDO decomposition plan. `src/session/durable-object.ts` (2022 lines) is about to have its object graph rebuilt by a composition root. The request path is already well covered by the existing integration suite (960 tests before this PR), but the thin spot is what happens when a callback lands on a **freshly reconstructed** DO instance: `ensureInitialized()` rebuilding the graph, and `getClientInfo()` rebuilding `ClientInfo` from `ws_client_mapping` when the in-memory cache is gone. Before this PR, nothing caught a refactor that broke either one. This is the safety net going in first, ahead of the refactor it is meant to protect. ## How it was tested Each test was verified to fail when the behaviour it covers is deliberately broken in `src/`, then `src/` was reverted. Confirmed red for: - removing `ensureInitialized()` from `SessionServer.onMessage` (tests 1 and 3 fail) - removing `ensureInitialized(false)` from `SessionServer.onScheduledDeadline` (test 2 fails) - removing the `close(ws, 4002, ...)` call from `SessionDO.getClientInfo` (test 4 fails) - changing `getClientInfo` to use `mapping.user_id` for both `userId` and `name` (test 3 fails on both fields) Green on this branch: - `test/integration/durable-object.test.ts`: 9 passed (5 pre-existing, byte-identical, plus 4 new) - full control-plane integration suite: 76 files / 964 tests, exit 0, no `EnvironmentTeardownError` - control-plane unit suite: 195 files / 3036 tests - `npm run typecheck -w @open-inspect/control-plane`: clean - `prettier --check` and `eslint`: clean on both changed files ### Expected log noise `ctx.abort()` makes workerd print two lines per run: ``` uncaught exception; exception = workerd/api/actor-state.c++:1142: failed: broken.outputGateBroken; jsg.Error: test: force eviction ``` This is a runtime log line, not a vitest failure. The suite still exits 0. Expect it in CI logs. ## Review follow-up applied The second commit removes the one timing dependency in the new code. The restored-socket helper originally slept a fixed 100ms waiting for frames and close events to cross the `WebSocketPair`; it now settles on an `until` predicate or on the close event, with the same 2000ms fallback `collectMessages` uses. `collectMessages` itself could not be reused because it only observes frames, and the 4002 case asserts on a close event. Confirmed the wait still has teeth: under the break-tests above the failing cases hit the 2000ms fallback rather than settling vacuously, and target-file runtime dropped from 529ms to 219ms. ## Deferred, with reasons - **`test/integration/tsconfig.json` reports 10 more errors than baseline** (834 to 844). Being precise since an earlier note in review claimed "no new errors", which was wrong. All 10 are the same pre-existing class already present at baseline: missing `@cloudflare/workers-types` globals and `cloudflare:test` members (`TS2304 DurableObjectStub`/`WebSocketPair`, `TS2339` `ctx` on `SessionDO`, `TS2305` on `cloudflare:test` exports, `TS7006` on `addEventListener` params), caused by that config setting `types: ["@cloudflare/vitest-pool-workers"]`. That tsconfig is referenced by no npm script and no CI job, and both `tsconfig.json` and `tsconfig.test.json` include only `src/**/*.ts`, so `npm run typecheck` does not cover these files at all. Zero CI impact, and fixing the config is out of scope for a test-only PR. - **Test 4 overlaps existing coverage.** `websocket-client.test.ts` already has "rejects typing before subscribing", which hits the same null-mapping branch, and the eviction is not load-bearing for it. Kept anyway: it was an explicitly requested scenario and it is the only test asserting the close reason string. - **Test 2 does not pin the `rehydrateAlarm = false` argument.** Not added, because `src/session/server.test.ts:357` already asserts `expect(ensureInitialized).toHaveBeenCalledWith(false)` directly, so the regression this would guard against is already caught. An integration-level proxy assertion would be strictly weaker and more brittle. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved reliability when sessions are restored after eviction or hibernation. * Ensured queued prompts continue processing after session recovery. * Ensured timed-out messages are correctly marked as failed after recovery. * Preserved client identity and presence details when reconnecting restored sessions. * **Tests** * Added integration coverage for session eviction, WebSocket restoration, alarm handling, message persistence, and client identity recovery. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com>
…ay#1583) ## Phase 1.5 of the SessionDO decomposition plan `SessionDO` handed itself out to its own collaborators through `=> this.<method>` closures, and a large share of those targeted **one-to-three-line forwarder methods** that did nothing but call a collaborator the DO already held. This PR deletes those forwarders and repoints every caller at the collaborator directly, plus hoists seven DO methods that only ever needed platform scalars into module-level pure functions. Mechanical and behaviour-preserving. No route, schema, or message-shape changes. ### Stacking note — read this This was written as a stacked PR on top of **ColeMurray#1578** (`refactor/break-lifecycle-message-queue-cycle`). **ColeMurray#1578 has since merged**, its branch was deleted upstream, and the merged version differs from the branch head this work was based on (the callbacks moved into `session/alarm/handler.ts` rather than a `setCallbacks` method). The branch has therefore been **rebased onto `main`** and this PR targets `main` directly. There is nothing left to merge first. The rebase required two adaptations to code that landed in the meantime, both in their own commits: - **ColeMurray#1577** added a terminal-session admission check calling `this.getSession()` in the sandbox WS upgrade path. Repointed to `this.sessionCoreRepository.getSession()`. - **ColeMurray#1577**'s `revalidates terminal state after asynchronous authentication` test spied on the DO's private `isValidSandboxToken`, which this PR hoists to a module function. The test now drives the same race from the pre-authentication sandbox read, which queues the cancel as a microtask that lands inside the token-hash await, and additionally asserts the response body so it can only pass via the post-authentication session read. Verified the guard still bites: moving that session read back above the await turns the upgrade into a `101` and fails the test. ### Numbers, and what they do NOT mean | | before | after | |---|---|---| | `durable-object.ts` lines | 2040 | 1843 (**-197**) | | `=> this.` occurrences in `durable-object.ts` | 127 | 119 (**-8**) | The line delta is real: 22 method bodies plus their doc comments are gone, and ~215 lines of that moved into small, separately testable modules. **The back-edge delta is small on purpose, and it does not mean the DO is decoupled.** The closures were retargeted, not removed: `getSession: () => this.getSession()` became `getSession: () => this.sessionCoreRepository.getSession()`. That is still a closure over `this`. What changed is the *shape* of the coupling — collaborators now reach a repository or a service instead of bouncing off a DO method that exists only to forward. The DO is still the composition root, still holds every collaborator, and 119 closures still resolve DO fields at call time. Removing that requires a real composition root, which is Phase 2b. Of the 119 remaining, 35 are the `routes` dispatch table (legitimate, explicitly out of scope) and 13 target genuine DO domain logic left for Phase 2a/3 (`ensureInitialized`, `handleSubscribe`, `handleSnapshot`, `handleSandboxAccess`, `applySessionTitleUpdate`, `ensureRepoId`, `getProviderAuthenticationError`, `getUserEnvVars`, `schedulePullRequestRefresh`, `loadMemberRepoSecrets`, `getClientInfo`, `handleWebSocketUpgrade`). ### Deleted — 15 forwarder methods `getSession`, `getSandbox`, `updateSandboxStatus`, `safeSend`, `broadcast`, `spawnSandbox`, `warmSandbox`, `triggerSnapshot`, `updateLastActivity`, `scheduleInactivityCheck`, `processMessageQueue`, `stopExecution`, `processSandboxEvent`, `pushBranchToRemote`, `handlePromptMessage`. All were `private` with no external callers. Every call site is repointed with the argument list preserved. Two carried doc comments their collaborators lacked (`stopExecution`'s synthetic-event contract, `pushBranchToRemote`'s wait-for-completion contract); those comments moved down to `SessionMessageQueue.stopExecution` and `SessionSandboxEventProcessor.pushBranchToRemote` rather than being lost. ### Hoisted — 7 methods to module-level pure functions | function | new home | |---|---| | `getPublicSessionId` → `resolvePublicSessionId` | `session/public-session-id.ts` | | `isValidSandboxToken`, `decryptStoredAccessValue`, `getSandboxDashboardUrl` → `resolveSandboxDashboardUrl` | `session/sandbox-access.ts` | | `resolveScmSettings` | `session/scm-settings-resolution.ts` | | `parseArtifactMetadata` | `session/artifact-metadata.ts` (existing) | | `safeParseTunnelUrls` | `session/tunnel-urls.ts` (existing) | `parseArtifactMetadataJson` and `parseTunnelUrls` are reused, not duplicated. Each hoisted helper got direct unit tests — they no longer need a Workers runtime, which is the point. The sandbox-token tests are load-bearing: nothing else in either suite covered hash-vs-plaintext precedence. `resolvePublicSessionId` dropped a `?? this.getSession()` fallback that was provably dead — all seven call sites either guard on `if (!session) return` or are typed non-nullable by their dependency contract. ### The constraint this change had to respect `this.lifecycleManager` is a lazy getter whose construction calls `createSandboxProviderFromEnv`, which **throws** on missing provider env. Every thunk stayed a thunk; only the resolution target changed. Confirmed mechanically with an AST pass over both files: construction-time `this.<member>` reads (constructor body, getter body, field initialiser — i.e. not inside an arrow) went 129 → 130, and the single new one is the *write* to `sandboxDashboardSettings`. One site got this wrong on the first pass and is fixed in `fix: keep warm-spawn getter read inside the background task`. Deleting the `async warmSandbox()` wrapper moved the getter read into argument position of `backgroundTasks.submit(...)`, converting a swallowed promise rejection into a synchronous throw that escaped `POST /internal/init` — a 500 on session create *after* the session, sandbox and participant rows were already committed. The read now happens inside the submitted promise. `sandboxDashboardSettings` is a new constructor-time snapshot of three plain optional `env` strings (`SANDBOX_PROVIDER`, `MODAL_WORKSPACE`, `MODAL_ENVIRONMENT`). A DO's `env` is fixed for the instance's lifetime, so this is observationally identical and cannot throw. ### Kept, with reasons - **The `routes` dispatch table.** Its closures are legitimate dispatch, not forwarding. Untouched. - **`createSourceControlProvider()`** — a literal one-liner, but it is not the target of any `=> this.` closure, so deleting it removes zero back-edges. It belongs to Phase 2b's lazy-getter removal. - **`getIsProcessing()`** — one line, but it adds a predicate over `getProcessingMessage()`; single internal caller, no back-edge. - **`fetch` / `webSocketMessage` / `webSocketClose` / `webSocketError` / `alarm`** — pure forwards to `this.server`, but they are the DurableObject platform contract. ### Tests Deletions of untested forwarders can't be TDD'd literally, so each deleted edge was checked for existing coverage first. Two had none that could see it, and both had a **success-shaped fallback** that hid the gap — a silent stand-in for either left all 975 integration tests green: - `triggerSnapshot` is fire-and-forget through `backgroundTasks.submit`, and its only unit test injects a `vi.fn()` for the dep, so it cannot see the DO's wiring. - `pushBranchToRemote` returns `{ success: true }` when no sandbox is connected — exactly what a dropped edge returns — and `create-pr.test.ts` never opens a sandbox socket. New `test/integration/session-do-collaborator-wiring.test.ts` pins six edges. Each was verified by mutation (replace the thunk with a plausible stand-in, confirm exactly one test fails, revert): | guard | mutation it kills | |---|---| | typing → `lifecycleManager.spawnSandbox` | no-op the spawn | | `execution_complete` → `lifecycleManager.triggerSnapshot` | `Promise.resolve(void reason)` | | create-PR → `sandboxEventProcessor.pushBranchToRemote` | fake `{ success: true }` | | init survives an unbuildable provider | read the lazy getter in argument position | | snapshot surfaces stored `tunnel_urls` | hardcode `tunnelUrls: null` | | snapshot falls open on a corrupt blob | (asserts the blob survived, so it can't pass vacuously) | The tunnel-URL cases deliberately leave the sandbox row in its terminal `failed` state rather than reviving it to `ready`. The snapshot reads `tunnel_urls` regardless of status, and `failed` is a dead status the lifecycle alarm early-returns on — reviving the row re-armed an alarm that clears tunnel URLs, which is what made an earlier draft of these tests flaky under load. Also deleted a vacuous `resolveScmSettings` test whose `prepare` spy hung off a `db` the call never received; it passed for every possible implementation. ### Verification - `npm run typecheck -w @open-inspect/control-plane` — clean (both tsconfigs) - `npm test -w @open-inspect/control-plane` — 198 files / 3061 tests passed - `npm run test:integration -w @open-inspect/control-plane` — 79 files / 975 tests passed, twice consecutively from a clean tree - prettier + eslint clean on all changed files ### Deferred - **`(reason) => this.lifecycleManager.triggerSnapshot(reason)` at the sandbox-event processor is not wrapped in an async IIFE like the warm-spawn site.** Deliberate. On that path the two statements immediately after it (`updateLastActivity`, `await scheduleInactivityCheck`) resolve the same getter and were *already* synchronous-throwing on the base branch, so a misconfigured provider produces the same failure with or without the wrapper. Adding an unexplained `async` there would be noise. - **`session-status-service.ts` carries its own `getPublicSessionId`** that lacks the DO-id fallback. Consolidating it is a semantic decision, not a mechanical one, and it is outside `durable-object.ts`. - **`getSessionUrl` still inlines `session.session_name || session.id`** (two-arm, no DO-id fallback). Routing it through `resolvePublicSessionId` would add a third arm — a behaviour change, not a hoist. - **No test asserts that a lazy getter is never forced during construction.** The integration config supplies `MODAL_API_SECRET` and `MODAL_WORKSPACE`, so `createSandboxProviderFromEnv` never throws in CI. The new "init survives an unbuildable provider" case covers the warm-spawn path specifically by shadowing the getter, but a general guard belongs with Phase 2b. - **Phase 2a/3 work** — the five domain responsibilities (connection auth, snapshot/read model, secrets assembly, sandbox access, title) are untouched, as is anything resembling a composition root. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Improved session and sandbox access with secure token validation, credential handling, and dashboard URL resolution. - Added fallback resolution for public session identifiers and source-control settings. - Added safer parsing for artifact metadata and tunnel URLs, with warnings for invalid data. - **Bug Fixes** - Improved handling of corrupted metadata, tunnel data, missing credentials, and session terminal-state races. - **Tests** - Expanded coverage across session, sandbox, source control, metadata, tunnel, and WebSocket behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Supersedes ColeMurray#1471 with its merge conflict resolved and the review findings addressed. - imports and re-imports managed skills from GitHub or GitLab repositories with pinned, review-before-write provenance - verifies the full mapped revision at confirmation time and ignores stale UI preview responses - rejects symlinks, submodules, oversized streaming blobs, invalid YAML, and provider-mismatched re-imports - exposes every bounded supporting file in the confirmation preview, including executable script contents - restricts installation-wide skill and profile administration to authenticated human users ## Verification - shared unit tests: 678 passed - control-plane unit tests: 3,099 passed - web unit tests: 1,212 passed - managed skill import integration tests: 16 passed - shared, control-plane, and web typechecks passed - control-plane and web lint passed - control-plane and web production builds passed - exact-head focused verification: 228 control-plane tests and 3 web tests passed <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Import managed skills directly from GitHub or GitLab repositories. - Preview source files, metadata, warnings, commit details, and content digests before importing. - Re-import existing skills with source tracking, unchanged-content detection, and revision protection. - Display repository provenance in the skills catalog. - **Bug Fixes** - Prevent stale import previews from overwriting current results. - Improve handling of repository paths, oversized files, invalid content, and conflicting skill names. - **Documentation** - Document repository imports, validation, provenance, re-import behavior, and related roadmap plans. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: MhAhmadAli <fa17-bsse-023@lgu.edu.pk> Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com>
## Summary - add understated creator attribution to each shared skill row - fall back to the canonical creator ID when no display name is available - keep assignment and creator metadata responsive on narrow screens - add focused catalog coverage for display-name and ID rendering ## Verification - `npm test -w @open-inspect/web -- src/components/settings/skills-settings/skills-catalog.test.tsx` - `npm run typecheck -w @open-inspect/web` - `npm run lint -w @open-inspect/web -- src/components/settings/skills-settings/skills-catalog.tsx src/components/settings/skills-settings/skills-catalog.test.tsx` - visually verified at 1440x900 and 390x844 viewports --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/5a1e635f559acbad94fee85eab3dd77e)* <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Skill catalog entries now display the creator’s name or ID alongside the assignment count. * Metadata wraps automatically for improved readability in narrow layouts. * When a creator name is unavailable, the catalog falls back to displaying the creator ID. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com>
## Context This is the commit-signing half of closed PR ColeMurray#1580. The bridge initializes commit signing before opening its WebSocket. The configuration fetch previously collapsed every `httpx.HTTPError` into one status-less `GitSigningError`, so terminal 401/403/404/410 responses were treated like transient network failures and retried indefinitely. ## Changes - preserve the HTTP response status on redacted commit-signing fetch errors - make retryability explicit on `GitSigningError` - classify network and non-terminal HTTP failures as retryable - classify terminal authorization/not-found responses, malformed payloads, invalid manifests, and local Git configuration failures as non-retryable - let the bridge consume the signing boundary’s retryability decision instead of owning broker-specific status policy - retain fixed error messages without response bodies, URLs, authorization headers, or secret details ## Validation - `PYTHONPATH=src uv run --extra dev ruff check src tests` - `PYTHONPATH=src uv run --extra dev pytest tests/test_git_signing.py tests/test_bridge_reconnection.py -q` (43 passed) --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/1ce4e4cf0c1f19445af1d3d28ba283f3)* <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved Git signing error handling to distinguish temporary failures from terminal failures. * Authentication, authorization, and unavailable signing responses now stop processing immediately instead of repeatedly reconnecting. * Temporary network and service errors continue to support retry behavior. * Error messages remain sanitized and do not expose sensitive upstream details. * **Tests** * Added coverage for terminal HTTP statuses, retryable failures, and non-retryable signing errors. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com>
…Murray#1585) ## What Extracts the user-env responsibility (~145 lines, 5 methods) out of `SessionDO` into two focused units: - **`UserEnvResolver`** (`session/user-env-resolver.ts`) — resolves the user-defined environment a session's sandbox receives (decrypted global/repo/environment secrets folded with precedence, plus the managed-provider env derived from the session's provider auth modes) and answers whether a model's provider has usable authentication in that environment. Public: `getUserEnvVars`, `getProviderAuthenticationError`. - **`resolveSessionRepoId`** (`session/repo-id-resolution.ts`) — resolves (and persists) the primary repo id for legacy session rows that predate `repo_id`. Split out per review: it is repository identity, not environment assembly. The two token-refresh services and `UserEnvResolver` receive the same capability, so the resolver carries **no `SourceControlProvider` dependency**. Behaviour-preserving; method bodies moved verbatim modulo dependency substitution. The name follows the codebase's existing vocabulary for exactly this data (`getUserEnvVars`, "user-defined secrets" vs system vars); "Env(ironment)" alone was avoided because capital-E Environments are a different feature (`environment_id`, `EnvironmentSecretsStore`), and `sandbox-env.ts` already names the SESSION_CONFIG wire contract. This is the follow-on to ColeMurray#1577/ColeMurray#1578/ColeMurray#1579/ColeMurray#1583: `SessionDO` cannot shrink by extraction alone (each extraction historically left a `() => this.x` back-edge behind at ~1:1), so the plan is to build an eager composition root next — and this unit is the **only** domain responsibility that composition root itself depends on (`SandboxLifecycleManager` needs `getUserEnvVars`, `SessionMessageQueue` needs `getProviderAuthenticationError`). It jumps the queue so the root can be built without reaching back into the DO. ## Design notes - **The SCM provider stays behind a thunk at the `resolveSessionRepoId` call sites, deliberately.** `createSourceControlProviderFromEnv` throws on reachable configs (invalid `SCM_PROVIDER`; `gitlab` without `GITLAB_ACCESS_TOKEN`; `bitbucket` unconditionally), and repo-id resolution only needs it for legacy rows — sessions that already carry `repo_id` never construct it. The composition-root PR is the right place to decide between upfront config validation + eager construction vs. keeping the deferral. - **`log` is a thunk, deliberately.** The DO replaces `this.log` with a session-scoped logger inside `ensureInitialized()`; a by-value capture pins whichever logger existed at construction (`SandboxRepository` has exactly this live defect today — its `sandbox.status.unrecognized` warnings lack `session_id`). - The DO gains one lazy getter (`userEnvResolver`) wired in the existing getter pattern; the four consumer edges (message-queue provider-auth error, two token-refresh repo-id edges, lifecycle `getUserEnvVars`) are rewired. - Env is passed as two scalars (`repoSecretsEncryptionKey`, `secretsCapEnforcement`), not the whole `Env`. ## Metrics - `durable-object.ts`: 1850 → 1719 lines. - `=> this.` count: 118 → 119 (**expected to go up by one net**: the rewired edges still thunk through the DO getter for now; the composition-root PR is what deletes the getters and thunks wholesale. This PR's job is moving the logic, not the wiring). ## Tests - **22 new unit tests** across `user-env-resolver.test.ts` (18) and `repo-id-resolution.test.ts` (4) — the first decomposition units testable without a Workers runtime. The resolver harness runs a **real** `SessionCoreRepository` over a fake DO `SqlStorage` and **real** secret stores over a fake `SqlDatabase` with real AES-GCM round-trips, so the tests pin the production throw/return surface, not the fakes. Its default `resolveRepoId` is the real function over a throwing provider thunk, mirroring production wiring. Coverage includes: the no-session and D1-missing surfaces, the no-encryption-key path, `undefined`-not-`{}` on empty env, the managed-broker source filter (secondary repos excluded unless environment-launched), all four repo-id branches, lazy repo-id resolution for a legacy primary member, an id-less secondary member contributing nothing, secrets-cap enforcement (fail-closed enforce → `SecretsCapExceededError`; `warn` mode proceeds and logs — fixture built from the exported size constants as three individually write-valid scopes whose sum exceeds the combined cap, with the arithmetic asserted inline), and both injection contracts (logger swap honored after construction; a secrets-only load never constructs the SCM provider). - Integration safety net: full control-plane integration suite green (80 files / 995 tests), including `session-secrets-fold`, `session-from-environment`, `do-internal-routes`, and the collaborator-wiring guards. - The two secrets integration files called the DO's (now deleted) private method directly via `runInDurableObject`; they now share one `Pick`-typed accessor helper (`test/integration/session-do-access.ts`) so a rename breaks the helper instead of silently passing. Every assertion is unchanged. Follow-up tracked separately: ColeMurray#1588 (broker policy currently keyed on `SecretSource.label`; moved verbatim here, structural fix deferred deliberately). Typecheck (both tsconfigs), full unit suite, and Prettier are green. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Session environments now combine global, repository, and environment secrets consistently. * Managed-provider credentials and authentication errors are surfaced through session environments. * Missing repository identifiers can be resolved and saved automatically. * Unavailable secondary repositories are skipped safely. * **Bug Fixes** * Improved handling of missing sessions and inaccessible repositories. * **Tests** * Expanded coverage for secret merging, provider authentication, repository resolution, persistence, and environment limits. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…uthentication (ColeMurray#1586) ## What Completes the sandbox-WebSocket admission hardening started in ColeMurray#1577. `isValidSandboxToken` awaits `crypto.subtle.digest` — a **non-storage** await — so the Durable Object input gate does not hold other events across it: a cancel, archive, stop, or credential rotation can land mid-hash. ColeMurray#1577 moved the *session*-status guard behind authentication onto a fresh read; this PR does the same for the remaining state checks: 1. The sandbox-status guard (`410 Sandbox is stopped`) moves from pre-auth to post-auth and now operates on a **fresh** `sandboxRepository.getSandbox()` re-read. 2. New consistency check: if the sandbox row's `modal_sandbox_id`, `auth_token_hash`, or `auth_token` changed while the request was suspended in token hashing → `403 Forbidden: Sandbox credentials changed`. 3. `SessionWebSocketManagerImpl`'s hibernation-recovery loop now closes (`1000, "Sandbox identity changed"`) a cached socket whose tag doesn't match the persisted sandbox ID, instead of just skipping it. Guard order after this PR: sandbox-ID mismatch `403` (pre-auth — pure synchronous comparison, nothing can go stale before it) → token `401` → fresh session read `410` → fresh sandbox read `410` → consistency `403`. Our production deployment has run exactly this ordering since Aug 18. ## Externally visible contract changes (both verified safe for the bridge) - Stopped sandbox + **invalid token**: `410` → `401`. - Stopped sandbox + **wrong X-Sandbox-ID**: `410` → `403 Wrong sandbox ID` (the stopped-check used to run before even the ID check). The only in-repo consumer is the sandbox bridge (`packages/sandbox-runtime/src/sandbox_runtime/bridge.py`): a non-101 upgrade raises `InvalidStatus`, and `bridge.py` funnels 401/403/404/410 into the same terminal `SessionTerminatedError` — one attempt, no retry, clean shutdown (`test_bridge_reconnection.py` pins all four). No retry-forever risk. Security-wise the unauthenticated surface **shrinks**: lifecycle state (`stopped`/`stale`) is no longer revealed to callers without a valid token; an unauthenticated probe now sees only `401`/`403`. ## Red-first evidence The three new integration tests were written first and run against the unmodified base: - sandbox stopped mid-auth → expected `410`, got `101` (pre-auth guard read the pre-flip row) - credentials rotated mid-auth → expected `403`, got `101` (no consistency check existed) - stopped sandbox + invalid token → expected `401`, got `410` (stopped-check ran pre-auth) All three fail on base and pass with the change; the pre-existing session-terminal race test passes unchanged (its assertions are untouched — only two comments that described the old ordering were updated, and its inline spy was folded into the shared `mutateSandboxDuringAuth` helper). The new websocket-manager assertion was likewise red on base (`close` never called) and green now. Also strengthened `sandbox-events.test.ts`'s duplicate-`execution_complete` test with four lifecycle-reconciliation assertions (snapshot trigger, activity bump, inactivity reschedule, queue drain) — pinning existing behavior. ## Review updates - Per review, the recovery loop now also **skips and closes untagged sandbox sockets** when a persisted sandbox ID exists (`43b4d0f5c`, red-first verified) — matching the production recovery loop exactly. The cached-socket per-call identity re-validation remains deliberately out of scope. - The deeper pre-persistence replacement race surfaced in review (a stale bridge authenticating between replacement start and `updateSandboxForSpawn` persisting) is **pre-existing** — the old pre-auth guard had the same window and production has run this exact ordering since Aug 18. This PR narrows admission races to that one window; closing it is a lifecycle-ordering change tracked as ColeMurray#1589 with two designed fixes. ## Known gaps, deliberate - The credentials-changed `403` emits no `ws.connect` log event (parity with the production ordering as deployed; a follow-up can add a `reject_reason` for observability). Typecheck, full unit suite, full integration suite (80 files / 998 tests), and Prettier are green. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved sandbox WebSocket reconnection handling when sandbox identity or credentials change during authentication. * Prevented stale or unidentified connections from being restored after sandbox identity changes. * Ensured stopped sandboxes and invalid credentials return the correct authentication response without exposing lifecycle details. * Improved reliability when sandboxes shut down during connection or authentication. * Duplicate execution completions now consistently trigger expected session updates and queued message processing. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…oleMurray#1587) ## What `BackgroundTasks.submit` now takes a **task factory** (`() => Promise<unknown>`) instead of an already-started promise, and invokes it synchronously inside try/catch. A synchronous throw while building the task — the canonical case being a lazy getter whose construction throws on missing provider env — becomes a logged `background_task.failed` instead of escaping the caller. This is the follow-up promised in the ColeMurray#1583 review: that PR had to keep a named `warmSandbox()` method alive purely as an async boundary, because with `submit(promise)` the safety of the call depended on the call-site expression shape. With the factory signature the boundary is structural, so this PR **deletes that method** (and its 11-line justification comment) and inlines `() => this.lifecycleManager.warmSandbox()` at the one call site. ## Why synchronous invocation (not `Promise.resolve().then(task)`) Under the old signature, the task expression always executed synchronously before `submit` was entered. Invoking the factory synchronously preserves that timing exactly at all 23 call sites; a microtask deferral would change execution order everywhere for zero benefit. The observable contract is identical either way: synchronous throws are absorbed and logged, `waitUntil` is only called with a real promise. A unit test pins the synchronous-execution semantics so a future "cleanup" to microtask deferral fails loudly. ## Scope - 23 production call sites across 11 files converted to factories; 7 test fakes updated to invoke the factory (so background work still runs in tests with the same timing as before). - Four sites passed a pre-created promise variable; three (`webhooks/github.ts`, `image-builds/save-hooks.ts`, `message-queue.ts`'s projection chain) moved creation inside the factory — for `save-hooks.ts` this also pulls the synchronous workflow construction inside the absorbed boundary, killing the exact bug class this refactor targets. - The fourth, `routes/repos.ts`, deliberately keeps one shared promise: the HTTP response awaits the **same** refresh run that `submit` keeps alive past client disconnect, so moving creation inside the factory would double-execute the SCM refresh. `submit(() => refresh)` + a comment; safe because `refreshReposCache` is an `async function` and cannot throw synchronously. - `test/integration/session-do-collaborator-wiring.test.ts` still passes both guards for the warm-spawn edge — the shadowed-getter read counter and the provider-env-throw test (`/internal/init` returns 200 with the throw absorbed and logged). Its mechanism comment now describes the new contract. ## Verification Red-first on the implementation's unit tests (4/4 failed against the old code with `TypeError: task.catch is not a function`, green after). Typecheck (both tsconfigs — noting `test/integration/**` is not typechecked, the converted paths there were exercised by running 7 integration files), full unit suite, full integration suite (80 files / 995 tests), Prettier all green. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved reliability of background operations by handling immediate and asynchronous failures. * Prevented background-task setup errors from interrupting primary request processing. * Preserved error logging across session, repository, image-build, webhook, and notification workflows. * **Refactor** * Background work is now deferred until scheduled, avoiding unnecessary early execution. * Updated related workflows and test coverage to support consistent task scheduling and failure handling. * Added shared testing support for verifying submitted tasks and captured failures. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary - skip the main CI workflow when a pull request or push changes only Markdown files or content under `docs/` - continue running full CI whenever documentation changes are mixed with code or configuration changes ## Motivation PR ColeMurray#1571 changed only `CHANGELOG.md`, but triggered all 14 CI jobs. This is the first, narrowly scoped improvement in a broader effort to make CI package-aware. ## Validation - `npx prettier --check .github/workflows/ci.yml` - `git diff --check` `actionlint` was not available in the local environment. --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/6a9584bdf48356904b0771920dcb9482)* <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated continuous integration triggers to skip documentation-only changes, reducing unnecessary workflow runs for Markdown and documentation updates. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com>
ColeMurray#1594) ## What Phase 2b+3 of the SessionDO decomposition. All construction and wiring moves out of `SessionDO` into `createSessionRuntime(platform, env)` (`session/components.ts`) — an eager, topologically-ordered composition root that returns a **narrow runtime surface**, not the graph: ```ts interface SessionRuntime { readonly log: Logger; readonly server: SessionServer<WebSocket, ClientInfo>; // onRequest / onMessage / onClose / onError / onScheduledDeadline readonly alarms: { rehydrate(): void }; readonly internals: SessionComponents; // integration-test introspection only } ``` - **`durable-object.ts`: 1716 → 111 lines.** The DO is now a pure Cloudflare adapter: it initializes the runtime once per activation and forwards the five platform callbacks. All 24 lazy getters, 23 memo fields, the routes table, the socket adapters, and every domain method are gone from it. - The domain logic that previously stayed behind is extracted along real ownership lines: - `SessionConnectionAuthenticator` (`connection-authenticator.ts`) — sandbox WS upgrade guards (the post-auth re-check block), client subscribe + token TTL, snapshot handoff, post-hibernation identity recovery; - `SessionSnapshotReader` (`snapshot-reader.ts`) — the session read model (snapshot route + subscribe payload, per-repo git state, environment-name enrichment); - `SessionAccessReader` (`sandbox-access-reader.ts`) — sandbox access credential decryption/authorization; - `SessionTitleService` (`title-service.ts`) — title normalization/persistence/broadcast/index-sync. - Repositories, services, and handlers are **local to the factory**. Production code cannot reach them: the DO touches only `log`, `server`, and `alarms`. `internals` is not the graph — it is the **enumerated set of integration-test seams** (8 fields: 7 live collaborators to spy on, plus the substitutable SCM provider cell), each with a test that reaches it inside a `runInDurableObject` callback (the pre-existing idiom — those tests run against a live DO whose graph the init request already built, so their seams are necessarily late-bound). Adding a seam means adding both the field and its consuming test. - The boundary is **governed, not conventional**: a scoped `no-restricted-imports` rule allows only `session/durable-object.ts` (and tests) to import the composition root — red-verified by tripping it from a sibling module. Together with the DO exposing nothing to reference, the planned Phase 5 back-edge rule becomes trivial. - **One session-scoped logger with live context**: the factory creates a single logger whose `session_id` is injected per emit through a latched resolver (`session-logger.ts` + `createLatchedPublicSessionIdResolver`). On a first-ever activation the id upgrades from the DO id to the public session name the moment `/internal/init` writes the row — for every component, however early it captured the logger. (Previously the id was frozen at logger creation; on genesis activations that meant the DO id for the whole activation.) ## The two provider factories stay deferred — deliberately `createSandboxProviderFromEnv` and `createSourceControlProviderFromEnv` **throw on reachable configurations** (missing provider credentials; invalid `SCM_PROVIDER`; GitLab without a token; Bitbucket). Constructing them eagerly would turn `/internal/init` — which works on such deployments today and is pinned by the wiring suite — into a 500 on every request. The deferral is confined to documented cells at the root: - the sandbox provider, behind `createDeferredSandboxProvider` (getter-shape adapter over `once()`, so the lifecycle manager's signature and its 84 test construction sites don't change, and optional-method presence probes stay truthful); - the SCM provider, behind `once()` in a **local cell**; consumer closures capture a stable local binding (no temporal dead zone, no read through the returned record). `internals.sourceControlProvider` is an accessor pair over that cell — the getter for introspection, the setter as the live-DO integration seam; - `resolveScmProviderFromEnv` / `resolveSandboxBackendName` name resolution (also throwing) resolved per use. `test/integration/session-components.test.ts` pins the invariant: the full graph assembles under a deployment where both factories and both name resolutions throw. ## Initialization `ensureInitialized()` builds into a local and **publishes last** — a throw during schema init or graph construction leaves the activation uninitialized, so the next event retries instead of dereferencing an undefined runtime. It also emits a one-line `do.init` event with the initialization duration; the dispatcher's per-request `init_ms` now reads ~0 because initialization happens before dispatch (the cold-start cost moved to `do.init` — deliberate observability change). `SessionServerDeps.ensureInitialized` is kept and threaded through as a declared factory input, so hibernation-restored callbacks self-heal with exactly the same contract (and `server.test.ts` stays untouched). ## Frozen-at-construction values became per-use thunks Two values used to be read at first getter touch — after `/internal/init` commits the row — and would have been frozen too early by eager construction: - `executionTimeoutMs` (honors `sandbox_settings`): `SessionMessageQueue` and the alarm handler take `getExecutionTimeoutMs: () => number`, resolved when a deadline is armed. Pinned by a two-dispatch freshness test. - the lifecycle manager's log-correlation id: `config.sessionId` → `config.getSessionId` thunk with a memoized derivation, sharing the same latched resolver as the session logger. Pinned by a log-context transition test. All three pins are **mutation-verified**: the intended regression mutants (first-use freeze, latch-the-fallback, constructor-time capture) each fail their test. ## Behavior notes (deliberate, small) - Unsupported `SANDBOX_PROVIDER`: the lifecycle manager now constructs; the same `Unsupported SANDBOX_PROVIDER` error surfaces at the spawn that needs the provider (absorbed at the background-task boundary). Strictly more requests work on such a deployment. - Log lines from all session components now carry a live `session_id` (upgrade mid-activation on genesis) — an observability improvement, not a wire-format change. - `do.request`'s `init_ms` reads ~0; `do.init` supersedes it (see Initialization). - `UserEnvResolver`'s `log` dep is a plain `Logger`; the call-time-logger contract test is deleted with the contract it pinned. ## Tests - **Integration: 81 files / 999 tests green**, including the eviction/hibernation suite and the WS auth-race suite, against the runtime shape. - Unit: 206 files / 3184 tests, including new suites for the session logger wrapper, the latched resolver, the deferred-provider adapter, and the title service. - Integration tests that pierced DO privates are consolidated behind one accessor (`componentsOf` in `session-do-access.ts`, honestly documented: this directory is never typechecked, so the casts are hand-synced). ## What this unblocks Phase 4 (collapse the five socket-port abstractions) and Phase 5 (the lint rule pinning that nothing the factory builds references the DO — now trivially true, since the DO exposes nothing to reference). The ColeMurray#1589 spawn-generation fence design now has its home in `SessionConnectionAuthenticator`. Part of the SessionDO decomposition plan (Wave A ColeMurray#1577/ColeMurray#1578/ColeMurray#1579, Phase 1.5 ColeMurray#1583, Wave B ColeMurray#1585/ColeMurray#1586/ColeMurray#1587). --- ## Review history **Round 1 (pre-open, 6-lens adversarial pass):** 4 findings confirmed and fixed — per-log-line SQL read in the manager's log derivation (→ latched resolver), missing pins for the per-use thunk contracts (→ mutation-verified tests), stale seam doc comment. **Round 2 (teammate review, Request Changes at `d1d6c385`):** all findings accepted and addressed at `9c6c01a4`: 1. *Init can poison an activation* → build-into-local, publish-last (above). 2. *Mutable back-edge / TDZ in the SCM thunk* → local cell + accessor pair; consumer closures never read through the returned record; `SessionRuntime` itself is immutable. One qualification kept from the discussion: pre-construction injection through factory inputs cannot serve the live-DO integration idiom (tests inject **after** the init request they triggered has built the graph), so one explicit, documented late-bound seam remains — on `internals`, not on the runtime. 3. *Service-locator surface / no ownership boundaries* → the narrow `SessionRuntime` above, one step further than the sketch: since the routes table and socket wiring also moved into the factory, the adapter needs only `log`/`server`/`alarms` rather than per-capability members. Connection auth, snapshot assembly, and sandbox access are extracted as owning classes. At `d5e791b7`, `SessionComponents` was additionally shrunk from the full graph (~41 entries) to the 8 test-reached seams, and the only-the-adapter-imports-the-root lint rule landed. 4. *(Non-blocking) genesis logger pins the DO id* → fixed for all components via the per-emit `session_id` injection, not just the lifecycle manager.
…ray#1602) ## What Follow-up to ColeMurray#1594, by team decision: the two provider factories now construct **eagerly at graph build**, deleting the deferral machinery the composition root carried (`createDeferredSandboxProvider`, `once()`, `tryResolveSandboxBackendName`, and the thunk parameters that existed only to defer throwing name resolution). Net −151 lines. ## Behavior change — deliberate On a misconfigured deployment (missing sandbox-provider credentials, unsupported `SANDBOX_PROVIDER`, invalid `SCM_PROVIDER`, GitLab without a token, Bitbucket), every session request now fails at `ensureInitialized()` — including `/internal/init` — instead of init succeeding and the error surfacing later as an absorbed `background_task.failed` at the first spawn or PR operation. Rationale: the environment must not run misconfigured; deployment-time validation is the gate for configuration, not runtime degradation. Failing at graph build also fails **before any DO session state is written**, so a misconfigured deployment leaves no partially-initialized DO state behind (the D1 index row from session-create is cleaned by the draft-expiry sweep, as with any init failure). What this reverses: the "init succeeds on a provider-less deployment" posture pinned since ColeMurray#1583. What it keeps: ColeMurray#1583's actual bug class stays fixed — `ensureInitialized` still builds into a local and publishes last, so a throw leaves the activation uninitialized and retryable, and **runtime** spawn failures (provider API down, quota) are still absorbed by the background-task boundary without failing init (test retained, reworded). ## Simplifications - `components.ts`: both providers and both name resolutions construct eagerly at the root; the getter-shape `createDeferredSandboxProvider` adapter, `once()`, and `tryResolveSandboxBackendName` are deleted. - The `SessionComponents.sourceControlProvider` seam is instance-typed (`SourceControlProvider`, was `() => SourceControlProvider`) — still an accessor pair over a local cell, so live-DO integration tests substitute stubs exactly as before (5 injection sites simplified). - `SessionMessageQueue` regains its pre-ColeMurray#1594 parameter: `scmProvider: SourceControlProviderName` as a value (the thunk existed only for deferral). - `SessionConnectionAuthenticator` takes `scmProviderName` as a value for the same reason. - Unchanged: `getExecutionTimeoutMs` and `getSessionId` stay thunks — they defer for **freshness** (honoring `sandbox_settings` persisted after construction; the public-session-id latch), not for misconfig tolerance. ## Tests - `test/integration/session-components.test.ts` inverted: it was the eagerness-tolerance pin ("the graph builds without constructing either provider"); it now pins fail-fast — a healthy env builds, and an unsupported `SANDBOX_PROVIDER`, missing modal credentials, or an invalid `SCM_PROVIDER` each throw at graph build naming the bad configuration. - `src/session/components.test.ts` (the deferred-adapter unit suite) is deleted with the adapter. - "keeps session init succeeding when the warm spawn cannot build its sandbox provider" → reworded to the invariant that survives: **runtime** warm-spawn rejections are absorbed and init still succeeds. - Unit 3180 / integration 1002 (81 files) all green; typecheck, eslint (including the only-the-adapter-imports-the-root rule), and prettier green. The integration env models a correctly configured deployment (modal creds + default GitHub SCM), so eager construction is exercised by every DO test. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Invalid or incomplete provider configurations are now detected during session startup instead of later during sandbox or pull request operations. * Provider initialization errors are surfaced earlier, making session startup behavior more predictable. * Runtime sandbox failures are handled separately from configuration errors. * **Tests** * Expanded integration coverage for fail-fast startup validation and provider configuration errors. * Updated session and pull request tests to reflect direct provider configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
ColeMurray#1603) ## What Phase 5 of the SessionDO decomposition plan, plus the small root deduplications that ColeMurray#1594/ColeMurray#1602 exposed. Zero behavior change (one log field dropped, see below). ### The back-edge boundary rule (Phase 5) The plan's original Phase 5 rule — "no `this.` inside arguments passed to a constructor in `durable-object.ts`" — became vacuous after ColeMurray#1594: the adapter passes no collaborators anywhere. What remains meaningful is the module-level back edge, and it's now governed: - **Only `src/index.ts` may import `session/durable-object`** (it exports the DO class to the runtime). Nothing the factory builds can hold a reference back to the DO, because nothing else can even name its module. - This completes the pair with ColeMurray#1594's rule that only the adapter may import the composition root. Both bans live in one `no-restricted-imports` block (base rule, so the repo-wide `@typescript-eslint/no-restricted-imports` shared-auth config still stacks). Flat-config replaces per-rule configs for overlapping file sets rather than merging them, so the general block carries both bans and `src/index.ts` re-declares the composition-root ban it is still subject to — the structure is commented in `eslint.config.js`. Red-verified in all three directions: a session module importing the adapter trips; `title-service.ts` importing the root still trips after the restructure; `index.ts` importing the root trips. ### Root deduplication `createSessionRuntime` constructed several identical things repeatedly; each is now one shared local: - `SessionIndexStore` — was constructed 4× (`db ? new SessionIndexStore(db) : null` at the terminal-message projection, status service, title service, message queue) - `SessionPullRequestStore` — was constructed 2× (PR refresh, PR creation) - the `resolveSessionRepoId` closure — was declared 3× (user-env resolver, OpenAI + xAI token refresh) - the terminal-message projection closure — was declared 2× (message queue, sandbox event processor) ### `init_ms` removed from `do.request` Since ColeMurray#1594, initialization happens before dispatch (the adapter builds the runtime on first event), so the dispatcher's per-request `init_ms` always reads ~0 — the one-line `do.init` event carries the real cold-start duration. The dead field and its clock read are removed; `duration_ms`/`handler_ms` are unchanged. (The dispatcher still calls `ensureInitialized()` — removing that dep is Phase 4's churn, where the server stack is already being touched.) ## Tests 3180 unit / 1002 integration green; typecheck, eslint, prettier green. `server.test.ts` touched only to drop the `init_ms` assertion and its fake-clock sample. Part of the SessionDO decomposition plan (ColeMurray#1577/ColeMurray#1578/ColeMurray#1579, ColeMurray#1583, ColeMurray#1585/ColeMurray#1586/ColeMurray#1587, ColeMurray#1594, ColeMurray#1602). Phase 4 (socket-port collapse) follows on top of this. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Improved session runtime efficiency by reusing shared session and pull-request services. * Centralized terminal-message recording for more consistent session behavior. * **Bug Fixes** * Simplified request timing information by removing initialization-time metrics from request logs. * **Tests** * Updated session server tests to reflect the revised request timing data. * Added validation for session import boundaries to help prevent configuration-related issues. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
ColeMurray#1604) ## What Phase 4 of the SessionDO decomposition — the socket-layer collapse. Stacked on ColeMurray#1603 (retarget to `main` after it merges). Net ~−230 lines and one fewer abstraction layer on every message the session sends. ## The finding that reshaped the phase The plan (§Phase 4) prescribed retiring `SocketRegistry`, `SandboxBroadcaster`, and the lifecycle manager's `WebSocketManager` "in favour of `SessionConnections`". Auditing the post-ColeMurray#1594 code inverted that: **`SessionConnections` was itself the parallel abstraction.** Of its six methods, four (`registerBrowser`, `registerSandbox`, `listParticipants`, `disconnectSandbox`) had **zero production consumers**; `InMemorySessionConnections` — the "working double" — was consumed only by its own test file; and the one non-messenger consumer (`SessionConnectionAuthenticator`) bypassed the interface entirely, depending on the concrete class for `createUpgradeSockets()`, which isn't even on the port. So the collapse goes the other way: - **`SessionMessengerImpl` now implements delivery directly over `SessionWebSocketManager`** — same interface (`broadcast` / `sendToSandbox`), same semantics byte-for-byte (including `SandboxDeliveryUnavailableError`, which moves to `messenger.ts`; `diffs/service.ts` catch repointed). The 13 messenger consumers see no change. - **Deleted**: `connections.ts`, `durable-object-session-connections.ts`, both their test files, and the dead 4-method surface. - `createUpgradeSockets()` moves to `SessionWebSocketManager` (socket creation lives with the socket registry); the authenticator's concrete `connections` dep is gone. - The hibernation ping/pong auto-response setup moves to the composition root (platform-global wiring; also keeps `SessionWebSocketManagerImpl` constructible in the node unit environment, where `WebSocketRequestResponsePair` doesn't exist). - `projectConnectedParticipants` moves to its only consumer (`presence-service.ts`) and is retyped against the shared `ParticipantPresence` (field-identical); the private `ConnectedParticipant` type dies. ## SandboxBroadcaster: tightened and satisfied by the messenger `SandboxBroadcaster.broadcast(message: object)` → `broadcast(message: ServerMessage)`. Every one of the lifecycle manager's ~25 broadcast payloads already typechecks against the shared union, so the root now passes the **messenger itself** as the broadcaster and the casting adapter literal is deleted. The interface (and the manager's constructor) is otherwise untouched — deliberately: `manager.test.ts` has 83 direct construction sites, and the interface is a legitimately narrow consumer-owned port. ## ensureInitialized unthreaded from the server stack `SessionServerDeps.ensureInitialized` and `SessionHttpDispatcherDeps.ensureInitialized` are removed, along with the calls at every entry point, and `SessionPlatform` drops the field. Since ColeMurray#1594, the adapter initializes the runtime before any server entry point is reachable — including callbacks delivered to a hibernation-restored instance, which reconstruct the runtime through the private `runtime` getter (and `alarm()`'s explicit call). The in-stack calls were structurally unreachable-before-initialized. The eviction/hibernation integration suite passes unchanged, which is precisely the scenario this threading existed for. ## What deliberately stays, and why - **`SocketRegistry` + the `ports.ts` interfaces** — the server stack (router, disconnect handler) is generic over `<Connection, Client>` and needs connection-addressed operations (classify/reply-to/close *this* socket) that a connection-anonymous delivery port cannot express; that genericity is what lets `server.test.ts` run the full routing/disconnect policy on plain string connections. `ports.ts` now carries a header stating this rationale. - **The lifecycle manager's `WebSocketManager` port + its 8-line adapter** — consumer-owned narrow port; retiring it is an 83-test-site churn for zero production delta. - **`message-queue.ts` / `sandbox-events.ts` stay on `SessionWebSocketManager`** — the plan's "point them at the port" doesn't survive the actual flows: prompt dispatch is capture-claim-send (the socket captured before the claim guarantees the send goes to the sandbox that existed at claim time — an identity property worth keeping after ColeMurray#1586), and `pushBranchToRemote`'s no-socket branch is a *semantic* outcome ("assume pushed manually"), not a delivery failure. A connection-anonymous port can't express either without changing edge behavior. The messenger's docstring states this boundary. ## Tests - Unit 3170 (203 files) / integration 1002 (81 files) — all green; typecheck, eslint (boundary rules included), prettier green. - `messenger.test.ts` rewritten against a registry fake, porting the live coverage from the deleted suites: authenticated-only fan-out, sandbox delivery, no-sandbox rejection, and send-failure rejection. - `server.test.ts`: `ensureInitialized` fake and assertions removed; behavior assertions unchanged. Part of the SessionDO decomposition plan (ColeMurray#1594, ColeMurray#1602, ColeMurray#1603). With this, the decomposition's phases are complete. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Improvements** - Improved session WebSocket connection handling and message delivery. - Added automatic ping/pong responses to support reliable connections during hibernation. - Improved sandbox message delivery with clearer handling when a sandbox is unavailable. - Improved participant presence tracking across multiple active connections. - Simplified session initialization for more consistent request, message, and disconnect handling. - Improved connection setup and routing for more reliable session communication. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
… write (ColeMurray#1606) ## What Closes ColeMurray#1589 with the issue's recommended fix: the **persist-first, two-phase spawn write**. `doSpawn()`, its prebuilt-image retry, and `restoreFromSnapshot()` all followed the same ordering: set in-memory flags, `await hashToken(...)` — a non-storage await, so the DO input gate admits other events — and only then persist the new identity. During that await the OLD sandbox row was still the persisted truth, so a slow bridge from a reconnect-allowed sandbox (e.g. `failed`, deliberately reconnect-allowed so slow boots self-heal) could authenticate against the old row and pass every ColeMurray#1586 guard: both the pre-auth read and the post-auth re-read saw the same stale row. Comparing two persisted snapshots cannot detect a transition that has started but not persisted. ## The fix All three sites now write in two phases: 1. **Phase 1 — synchronous, before the first non-storage await**: `updateSandboxForSpawn` persists the replacement identity (`status: "spawning"`, new `modal_sandbox_id`) with `auth_token_hash: ""` — no token can match it. DO SQLite writes are synchronous, so this is visible before the input gate can open. A stale bridge arriving in what used to be the window now fails the sandbox-ID check (403) or the token check (401), or the ColeMurray#1586 credentials-consistency re-check (403). 2. **Phase 2 — after hashing**: a new narrow `SandboxStorage.updateSandboxAuthTokenHash()` publishes the real hash. The hash-less gap is unobservable: the provider has not been invoked yet, so no bridge holding the new identity can exist. Accepted caveat, per the issue: a spawn failure after phase 1 leaves the old credentials invalidated — replacement only starts when the old sandbox is being discarded, and failure paths already mark `failed` and re-spawn. ## Red-first race test Per the issue's requirement, the interleaving is reproduced before being fixed: `manager.test.ts` mocks `hashToken` with a pass-through gate (default open — the other 118 tests run unchanged against the real implementation), holds the next call open, starts a spawn, and asserts the **mid-window persisted row** — exactly what a stale bridge's admission read sees. On the previous code both cases fail with the old identity still visible (`expected 'sb-old' not to be 'sb-old'`); with the fix, the row already carries the new `modal_sandbox_id`, an emptied hash, and `spawning`. After release, the tests pin that phase 2 published a real hash and that the reservation write strictly precedes it. Both the fresh-spawn and snapshot-restore paths are covered; the image-retry path uses the same two-phase ordering (its existing `updateSandboxForSpawn`-call-count assertions are unchanged). ## Notes - Fix option 2 from the issue (the spawn-generation fence in the admission path) is superseded — revisit only if two-phase persistence proves insufficient. - `SandboxStorage` gains one method; implementers are the composition-root adapter and the shared test mock. Battery: 3172 unit (203 files) / 1002 integration (81 files), typecheck, eslint, prettier — green. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Security Improvements** * Improved sandbox startup and snapshot restore authentication handling. * Sandbox credentials are invalidated before replacement credentials are generated, preventing stale authentication attempts during transitions. * New authentication hashes are persisted only after generation completes and are tied to the active sandbox identity. * **Bug Fixes** * Improved protection against authentication race conditions during sandbox creation and snapshot restoration. * Prevented superseded sandbox launches from publishing outdated credentials or incorrectly marking the newer launch as failed. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…gest closure bags (ColeMurray#1608) ## What First PR of the deps-style normalization campaign (follow-through on the ColeMurray#1594–ColeMurray#1604 decomposition): replace the composition root's three biggest closure-bag literals with composition classes, per the house deps standard from the ColeMurray#1045-series (pass collaborators directly with full types; give a closure group that shares collaborators a named class). Behavior-preserving — no port changes, no call-flow changes. ## Changes - **`DurableObjectSandboxStorage`** (new `session/sandbox-lifecycle-adapters.ts`) implements the lifecycle manager's `SandboxStorage` port over its four real collaborators: `SandboxRepository`, `SessionCoreRepository`, `UserEnvResolver`, and the secrets encryption key. Replaces the 28-property literal in the root. The encrypt-before-store rule (code-server/VNC/ttyd secrets), previously copy-pasted three times inline, is one private `encryptIfConfigured` method. - **`LifecycleSocketAdapter`** (same file) implements the manager's `WebSocketManager` port over `SessionWebSocketManager` — the name translation and the no-socket send branch get a typed home instead of a literal. - **`SessionClientCommandFacade`** (new `session/client-command-facade.ts`) implements the message router's `SessionClientCommands<WebSocket, ClientInfo>` port with the four services as constructor deps. The port itself stays generic — that genericity is what lets the server stack unit-test over string connections, so the facade is the production binding, not a port rewrite. The router's client-message type aliases are now exported (they are referenced by the exported port, so naming them outside the module was already implied). Net: 39 function-valued props removed from `components.ts`; the root now constructs objects in these three spots instead of authoring behavior inline. ## Tests New `sandbox-lifecycle-adapters.test.ts` covers the pieces with real logic, which previously lived untested inside the root literal: the encrypt-when-configured branch (round-trips via `decryptToken`), the plaintext-passthrough branch, the repository-shape defaults (`baseBranch` → `"main"`, missing row → `baseSha: null`), the `setLastSpawnError` → `updateSandboxSpawnError` rename, and both `sendToSandbox` branches. Pure forwards stay covered through the manager and server suites. ## Queue context Next in the campaign (separate PRs): handler deps-bags → classes (normalizing the 7-factory/5-class split), vestigial thunk removal (`getLogger: () => log` first), and the `test/integration` typecheck spike. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Refactor** - Improved session command handling for prompts, execution controls, typing indicators, presence, subscriptions, and history. - Improved sandbox lifecycle and WebSocket handling for more consistent session connectivity. - **Security** - Sandbox access credentials can now be encrypted when configured, while retaining compatibility with existing setups. - **Tests** - Added coverage for credential storage, sandbox startup errors, repository behavior, and WebSocket communication. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…lve the storage middle-man (ColeMurray#1609) ## What Campaign item 2, combining two agreed decisions: **the secrets encryption key is required** (it always was operationally — Terraform declares it with no default — but the code treated it as optional and silently fell back to storing plaintext), and **the storage middle-man from ColeMurray#1608 is dissolved** (its ~25 one-line pass-throughs were the smell that prompted the design discussion). ## Encryption key is required - New `requireRepoSecretsEncryptionKey(env)`: the session graph throws at construction when the key is absent (the ColeMurray#1602 eager posture — a misconfigured deployment fails every request at initialization instead of running degraded), and the five MCP-server routes validate the same way. - Every plaintext-**write** fallback is deleted: the sandbox access-secret stores, `McpServerStore`'s keyless branch, and `UserEnvResolver`'s "skip secret loading" branch. `isManagedSecretsConfigured` reduces to `Boolean(db)`. - Plaintext-**read** fallbacks stay: pre-encryption legacy rows still decrypt-or-degrade exactly as before (`McpServerStore`'s catch fallback, access values resolving to null on decrypt failure). - The integration environment already provides a test key in its miniflare bindings, so no test-infra changes were needed. ## Encryption is owned by persistence; the middle-man is gone - `SandboxRepository` takes the key at construction and encrypts code-server/VNC/ttyd secrets inside its write methods — the same pattern the D1 stores already use. No caller can persist an access secret in the clear, structurally. - The manager's conflated port is **split into two roles** — the root cause behind both the ColeMurray#1608 forwarding layer and an interim inheritance design. `SandboxStorage` shrinks to the sandbox-row contract, which `SandboxRepository` now satisfies **structurally** (no adapter, no subclass, and no manager-port import in the repository — the structural check happens at the composition boundary). The three session-context reads become their own `SessionContextReader` port, implemented by a small `LifecycleSessionContext` facade over `SessionCoreRepository` + `UserEnvResolver` — an honest adapter: it spans two collaborators and owns the repository-shape defaults. `DurableObjectSandboxStorage` is deleted. - The shared test mock already implements both ports, so the manager's test harness changes are mechanical: the same fake is passed for both parameters at every constructor site. - `updateSandboxSpawnError` is renamed `setLastSpawnError` to match the port vocabulary, removing the last name translation. ## Tests Encryption round-trips (via `decryptToken`) now live in `sandbox-repository.test.ts` with the logic; the adapter tests pin the context mapping and the inheritance wiring ("sandbox writes hit SQL with no forwarding layer"). Deleted-behavior tests are deleted with their behavior: the keyless verbatim-read test, the resolver's skip-secret-loading test, and ColeMurray#1608's synchronous-keyless-persist test (that branch no longer exists — with the key required, every secret write takes the same WebCrypto await it always took on real deployments). `McpServerStore` tests construct keyed; their plaintext-seeded rows now exercise the legacy-read fallback, which is exactly what such rows are. ## Behavior change (intended) A deployment without `REPO_SECRETS_ENCRYPTION_KEY` now fails loudly at session initialization and on MCP routes, instead of silently persisting secrets unencrypted. Valid deployments are unaffected. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Security** * Repository secrets encryption is now required for control-plane operations. * Sandbox passwords, tokens, credentials, and stored environment secrets are encrypted before persistence. * Encryption keys are strictly validated for required format and length. * **Bug Fixes** * Improved handling of unavailable or empty stored secrets. * Reduced unnecessary decryption errors for empty credentials. * Improved sandbox error reporting. * **Refactor** * Streamlined sandbox lifecycle and session-context handling for more consistent behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary - move the six Python CI jobs into a dedicated `CI (Python)` workflow - keep the seven Node.js/TypeScript jobs in `CI (TypeScript)` - trigger each workflow only for its package and root-tooling dependency surface - preserve the Markdown-only exclusions added in ColeMurray#1590 ## Motivation The main CI workflow currently runs both ecosystems for every code change. This split prevents Python-only changes from allocating TypeScript runners and TypeScript-only changes from allocating Python runners, while preserving all existing job commands and dependencies. This is the ecosystem-level step before introducing narrower package-aware filtering in follow-up PRs. ## Validation - `npx prettier --check .github/workflows/ci.yml .github/workflows/ci-python.yml` - parsed both workflows and verified all 13 original job definitions remain present - `git diff --check` `actionlint` and Go were unavailable in the local environment. The repository-wide `npm run format:check` also reports a pre-existing formatting issue in `.opencode/package.json`; both changed workflow files pass their targeted formatting check. --- *Created with [Open-Inspect](https://open-inspect-prod.vercel.app/session/6a9584bdf48356904b0771920dcb9482)* <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Added dedicated continuous integration checks for Python linting, formatting, type checking, and tests. * Updated TypeScript validation to run through a dedicated workflow. * Refined workflow triggers to focus on relevant code and configuration changes, excluding documentation-only updates. * Expanded validation coverage for runtime, deployment, and infrastructure changes. * Added concurrency controls to cancel outdated runs and strengthened workflow security settings. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cole Murray <2492022+ColeMurray@users.noreply.github.com>
Terraform Validation Results
Pushed by: @jasoncuriano, Action: |
ScottKirschner
approved these changes
Aug 26, 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
7 participants
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 tod002c486.upstream/main— no fork-local changes includedpackage-lock.jsonleft untouched (npm's only local change wasdev→devOptionalflag churn, reverted)Verified locally before push:
npm run build(all packages)npm run typechecknpm test -w @open-inspect/control-planedist/index.jsartifacts🤖 Generated with Claude Code