refactor(bot): Extract platform-agnostic tool system by RSO · Pull Request #1 · Kilo-Org/cloud

RSO · 2026-02-04T08:15:15Z

Summary

This PR introduces a tool registry system that enables dynamic tool loading based on the owner's integrations, laying the groundwork for:

Supporting multiple chat platforms (Discord, Teams) beyond Slack
Adding new tool integrations (GitLab, Sentry, Axiom) without modifying core bot logic

Changes

New Tool System (`/src/lib/bot/tools/`)

types.ts: Core type definitions (BotTool, ToolResult, ToolExecutionContext, RequesterInfo)
registry.ts: Global tool registry with registerTool, getTool, getAllTools
tool-loader.ts: Dynamic tool loading based on owner's active integrations
implementations/spawn-cloud-agent.ts: Extracted spawn_cloud_agent tool
index.ts: Barrel export with auto-registration

Updated `slack-bot.ts`

Removed ~180 lines of hardcoded tool code
Now uses getToolDefinitionsForOwner() to dynamically load tools
Uses getTool() from registry to execute tool calls
Tools only available if owner has the required integration (e.g., github)

Architecture

Tool Registry (startup)
    └── spawn_cloud_agent (requiredIntegration: 'github')
    └── [future tools...]

Request Flow:
    1. getToolDefinitionsForOwner(owner)
       └── Check owner's integrations
       └── Filter tools by requiredIntegration
    2. LLM calls tool
    3. getTool(name).execute(args, context)

Testing

Manual testing of Slack bot with spawn_cloud_agent tool
Verified tool is only available when owner has GitHub integration

PR Stack

This is PR 1 of 7 in the Slack bot tech debt reduction initiative:

Tool System (this PR)
Bot Core extraction
Platform Adapter interface
Slack Adapter implementation
Simplified Webhook routes
Context Provider system
Generic logging with migration

Introduce core type definitions for the bot tool system: - ToolResult: result returned from tool execution - RequesterInfo: user attribution info for PRs - ToolExecutionContext: context passed to tool execution - BotTool: main interface for tools with dynamic loading support

Add global tool registry for managing bot tools: - registerTool: register a tool at module load time - getTool: look up a tool by name - getAllTools: get all registered tools - hasRegisteredTool: check if a tool exists - getRegisteredToolCount: get count for debugging

Add tool loader that dynamically loads tools based on owner's integrations: - getToolsForOwner: returns tools available to an owner - getToolDefinitionsForOwner: returns OpenAI tool definitions - Tools with requiredIntegration only available if owner has that integration

Move spawn_cloud_agent tool from slack-bot.ts to dedicated implementation: - Extracted tool definition and execution logic - Added BotTool interface implementation - Set requiredIntegration to 'github' for dynamic loading - Returns ToolResult with sessionId in metadata - Renamed logging prefix from [SlackBot] to [SpawnCloudAgent]

Create index.ts that: - Re-exports all types, registry, and loader functions - Auto-registers spawn_cloud_agent tool on module import - Provides usage documentation in JSDoc

Update slack-bot.ts to use the new tool system: - Import tools from @/lib/bot/tools instead of inline definitions - Dynamically load tools based on owner's integrations - Use getTool() from registry to execute tool calls - Build ToolExecutionContext with requester info for tools - Remove ~180 lines of tool-specific code (moved to tools module) This enables: - Dynamic tool availability based on owner's integrations - Easy addition of new tools without modifying slack-bot.ts - Consistent tool interface across different tools

github-actions · 2026-03-07T00:51:06Z

This Pull Request has been inactive for 30 days and will be closed in 7 days if no further activity occurs. Please update or close this PR if it is no longer relevant.

…review fixes - Fix #1: Add WHERE clause to updateConfig (config.ts accepts wastelandId param) - Fix #2: Update router.ts and ownership.ts imports from stub to Wasteland.do - Fix #3: initializeWasteland/storeCredential/updateConfig now return records - Fix #4: Implement all 6 missing RPC methods (updateMember, connectTown, disconnectTown, listConnectedTowns, getWantedBoard, refreshWantedBoard) - Fix #5: Synchronous initializeDatabase(), no ensureInitialized/initPromise - Fix #7: Delete WastelandDO.stub.ts New files: - 5 table definitions under db/tables/ (config, members, credentials, connected-towns, wanted-cache) - 5 sub-modules under dos/wasteland/ with business logic - Wasteland.do.ts: thin DO class delegating to sub-modules

…raffic Review fix #1. `skipBalanceCheck` only skipped the preflight zero-balance rejection; it did not mark the usage as free, so `processTokenData` would still record full upstream cost into `microdollar_usage.cost` and the insert CTE would increment `kilocode_users.microdollars_used`. A zero-balance user could be charged real upstream cost on a preview experiment and pushed negative. Adds an explicit `provider_funded` flag to `MicrodollarUsageContext` and treats it the same as `user_byok` in the cost-zeroing branch (zeroes `cost_mUsd` and `cacheDiscount_mUsd` while preserving `market_cost` for reporting). The route handler sets the flag for experimented requests, alongside the existing `skipBalanceCheck`.

…ge-limit guard Reorders the conjuncts on the usage-limit guard so the salient check reads first and `skipBalanceCheck` is clearly an override flag, not the gating condition. Pure reorder — boolean evaluation is unchanged. Addresses comment #1 on #3325.

* feat(gateway): capture x-kilo-request and x-kilo-session headers Phase 2 of the model-experiments rollout. Captures the per-message client request id (used as the join key for PostHog feedback and for `model_experiment_request.client_request_id` on experimented traffic) and accepts `x-kilo-session` as a fallback session id when `x-kilocode-taskid` is absent. Also extends MicrodollarUsageContext with the experiment routing fields (`modelExperimentVariantVersionId`, `modelExperimentAllocationSubject`, `experimentPromptCapture`, `clientRequestId`) so later phases can plumb them through without touching every usage-context construction site. * refactor(gateway): extract buildDirectProvider used by custom_llm path Phase 3a of the model-experiments rollout. Pulls the kilo-internal/... (custom_llm2) Provider construction into a shared `buildDirectProvider` helper under `apps/web/src/lib/ai-gateway/experiments/`. The experiment routing path added in Phase 3b will consume the same helper, so direct-to- upstream traffic for custom LLMs and experiments share one implementation. Behavior is unchanged for custom_llm callers. * feat(gateway): add experiment variant picker and routing-cache loader Phase 3b of the model-experiments rollout. Adds `apps/web/src/lib/ai-gateway/experiments/pick-variant.ts` with: - `isPublicIdExperimented(publicId)` — fast Redis-backed membership check with Postgres fallback, returning `unavailable` when both cache and DB are inaccessible so the gateway can fail closed. - `getRoutingExperimentForPublicId(publicId)` — Redis-cached resolved experiment for a public id, including a single Postgres `DISTINCT ON (variant_id)` query that picks each variant's current `model_experiment_variant_version` and decrypts `encrypted_api_key` once at cache-build time. Returns `none` (no routing-relevant experiment), `experiment` (active or paused), or `unavailable` (config/decrypt/validation failure or BYOK_ENCRYPTION_KEY missing). - `pickModelExperimentVariant(input)` — deterministic picker using `getRandomNumber`. Allocation subject precedence is user → machine → ip; missing all subjects returns `unavailable`. Variants are walked by id-asc cumulative weight so live label edits never rebucket users. The `upstream` JSONB blob is validated by ExperimentUpstreamSchema before use; partner-issued api keys live only in the encrypted column and the cached payload, never in tRPC responses. * feat(gateway): route experimented public ids to direct upstream Phase 3c + 3d of the model-experiments rollout. `getProvider` now returns a discriminated `GetProviderResult` union (`provider` | `not-found` | `unavailable`) and adds a new branch for experimented public ids that runs after the BYOK checks and before kilo-internal/custom_llm and the kiloExclusiveModels lookup. Active experiments produce a direct provider built via `buildDirectProvider` plus experiment routing metadata; paused experiments map to a local model-unavailable response so traffic doesn't silently fall through to default routing; cache/DB/config failures map to a 503 temporarily-unavailable response. Experiment traffic uses three new flags on the provider result: `skipBalanceCheck` (preview traffic is provider-funded), `skipProviderPin` (the partner endpoint is selected by the variant version, not by the org config), and `skipKiloExclusiveModelSettings` (which `applyProviderSpecificLogic` honors so the registry does not override the variant's `internal_id`). Server-side organization model/data-collection policy still runs against the public id. Also excludes experimented preview public ids from `kilo-auto/free` candidate sets so they are never silently selected on a user's behalf — users only ever reach the experiment path by opting in to the dedicated preview model id. The route handler captures `x-kilocode-machineid` once into `machineIdHeader` and threads it (plus the resolved client IP) into `getProvider` so the variant picker can fall back from userId to machineId to clientIp. * feat(r2): add experiment-prompts content-addressed storage helper Phase 4a of the model-experiments rollout. New helper module `apps/web/src/lib/r2/experiment-prompts.ts` exposes `putPromptIfAbsent(content)` (HEAD-then-PUT under a sha256 hex key for automatic dedup) and `getPromptByHash(sha)` (admin out-of-band reads). The bucket comes from a new env var `R2_EXPERIMENT_PROMPTS_BUCKET_NAME` exported from the existing R2 client module; per-environment buckets are `kilo-experiment-prompts-dev` and `kilo-experiment-prompts-prod`. The helper is wired up but not yet called from the request hot path — Phase 4b will add the bounded prompt capture and attribution writer that consumes it. * feat(gateway): persist experiment attribution after microdollar write Phase 4b + 4c of the model-experiments rollout. Adds `apps/web/src/lib/ai-gateway/experiments/persist.ts` with two exports: - `buildExperimentPromptCapture(request)` produces a bounded capture of the canonical post-`transformRequest` upstream body, splitting out the leading `role: 'system'` message for chat_completions and tail-truncating either side at 4 MB. The capture is stored on `MicrodollarUsageContext` so the async `after()` path never retains a reference to the full uncapped request body. - `persistExperimentAttribution(input)` writes one row into `model_experiment_request` after running two parallel content-addressed R2 puts via `putPromptIfAbsent`, using `__absent__` / `__failed__` sentinels for missing/failed sides. Errors are reported to Sentry and swallowed — the billing write must succeed independently of analytics. `processUsage.ts` now surfaces the inserted `{ usageId, createdAt }` out of `logMicrodollarUsage` and `processTokenData` so the `accountForMicrodollarUsage` after-hook can chain `persistExperimentAttribution` on the same row. The route handler calls `buildExperimentPromptCapture` after provider-specific transforms and stashes the result on the usage context only for experimented requests. * chore(experiments): drop unused MicrodollarUsageContext import * docs(plan): update experimental-models-1 status for PR #3325 Marks Phases 2 + 3 as merged-pending against PR #3325 and Phase 4 as in-progress with 4a/4b/4c on the same PR. 4d (the `model_experiment_request_stats` view) is now explicitly deferred until a real report consumer needs it. Also records the concrete output of each phase as it landed. * fix(experiments): zero billable cost for provider-funded experiment traffic Review fix #1. `skipBalanceCheck` only skipped the preflight zero-balance rejection; it did not mark the usage as free, so `processTokenData` would still record full upstream cost into `microdollar_usage.cost` and the insert CTE would increment `kilocode_users.microdollars_used`. A zero-balance user could be charged real upstream cost on a preview experiment and pushed negative. Adds an explicit `provider_funded` flag to `MicrodollarUsageContext` and treats it the same as `user_byok` in the cost-zeroing branch (zeroes `cost_mUsd` and `cacheDiscount_mUsd` while preserving `market_cost` for reporting). The route handler sets the flag for experimented requests, alongside the existing `skipBalanceCheck`. * docs(plan): record stale membership cache as next-step follow-up Captures the review-#4 finding from PR #3325: no TTL on the EXPERIMENTED_PUBLIC_IDS_REDIS_KEY plus best-effort/swallowed invalidation can leave a stale list indefinitely. Documents the agreed fix (add a TTL so the cache self-heals via the existing DB fallback path) and defers it to a follow-up PR. * fix(experiments): refuse experiment routing when org disables data collection Review fix #2. Direct experiment upstreams bypass the gateway-pinned `data_collection` setting — that setting only applies when the request is forwarded through OpenRouter or Vercel with a pinned provider config. For experimented public ids the upstream is selected by the variant version, so the org's preference would be silently ignored while we still capture prompts to R2 for partner evaluation. When an org sets `data_collection: 'deny'` and the request resolves to an experiment, return the existing `dataCollectionRequiredResponse()` instead of routing. The existing model deny list still runs first via `checkOrganizationModelRestrictions`. Anonymous users and non-org authenticated users have no settings and continue to route normally. * fix(experiments): cache encrypted upstream; decrypt only on pick Review fix #3. The Redis per-public-id experiment cache previously held decrypted partner-issued api keys for up to 10 minutes. That defeated the BYOK_ENCRYPTION_KEY fail-closed behavior: removing or rotating the env var did nothing for warm caches because `getRoutingExperimentForPublicId` returned the cached payload straight to the picker. The PR verification claim ('disabling BYOK_ENCRYPTION_KEY returns 503') only held on cold caches. Refactors the cache to hold CIPHERTEXT exclusively. Decryption + the missing-key check now run inside `pickModelExperimentVariant`, on the chosen variant only, on every request. Three knock-on benefits: 1. Key rotation is effective immediately on the next request; warm caches no longer serve stale plaintext keys. 2. We decrypt one variant per request instead of N at cache-build time. 3. A Redis dump no longer leaks usable api keys. Per-request cost is one AES-256-GCM decrypt (microseconds), which is negligible against an LLM round-trip. * fix(experiments): cap prompt capture by UTF-8 bytes, not UTF-16 units Review fix #6. SYSTEM_PROMPT_CAP_BYTES / REQUEST_BODY_CAP_BYTES were checked with `string.length` and trimmed with `string.slice`, which both operate on UTF-16 code units. For non-ASCII content the actual UTF-8 byte payload uploaded to R2 could be up to ~3× the intended cap (a single CJK char is 1 UTF-16 unit but 3 UTF-8 bytes; an emoji is 2 UTF-16 units but 4 UTF-8 bytes), so the cap was not bounding what the plan claimed it bounded. Slicing UTF-16 code units could also split a surrogate pair, leading the next UTF-8 encode to emit U+FFFD and breaking the deterministic-prefix promise (truncated content would hash differently across runtimes). Adds `truncateToUtf8Bytes(s, maxBytes)` which encodes once, walks back to a UTF-8 codepoint boundary using the 0b10xxxxxx continuation-byte rule, and returns valid UTF-8 with bytes <= maxBytes. Replaces the three call sites in `buildExperimentPromptCapture` to measure with `Buffer.byteLength` and truncate via the new helper. * test(experiments): cover picker, fail-closed paths, and UTF-8 truncation Adds two test files covering the new experiment routing surface: - `persist.test.ts` (14 cases) — `truncateToUtf8Bytes` boundary cases (multi-byte CJK, surrogate-pair emoji, exact alignment, zero cap, determinism); `buildExperimentPromptCapture` extraction (leading vs mid-conversation system message, serialization of structured content, UTF-8-byte cap enforcement for both system and body sides). - `pick-variant.test.ts` (13 cases) — `isPublicIdExperimented` positive/negative + paused-also-counted; picker determinism for the same userId; api-key decryption returns the right partner secret per variant; allocation-subject precedence user > machine > ip; `unavailable` when no allocation subject; `not-found` for paused so traffic doesn't silently fall through; hot-swap preserves the variant_id slot while serving the new variant_version_id; weighted-distribution sanity (1:3 split lands ~25%); historical attribution via the old version row survives a hot-swap; completed experiments stop routing. Full suite: 373 passed (up from 371). * fix(experiments): honor request-level data-collection opt-out Follow-up to review fix #2. The previous commit only refused experiment routing on the org-level setting (`settings.data_collection === 'deny'`). A caller can also opt a single request out of training/data-retention via OpenRouter's request-body fields `provider.data_collection: 'deny'` or `provider.zdr: true`. Direct experiment upstreams ignore those fields entirely (we don't go through OpenRouter), but the prompt-capture path was still uploading to R2 for partner evaluation, violating the caller's stated intent. Refuses experiment routing with `dataCollectionRequiredResponse()` when `!isFreePromptTrainingAllowed(body.provider)` regardless of org/anon/BYOK status. The check runs unconditionally right after `getProvider` returns so it covers anonymous and non-org authenticated users too. Adds a unit test for `isFreePromptTrainingAllowed` covering all opt-out shapes (`data_collection: 'deny'`, `zdr: true`, `zdr: false`, both, neither, undefined) so a regression cannot silently allow capture for users who explicitly opted out at the request scope. Also tightens the `GatewayRequest` cast in `persist.test.ts` per the test-file as-cast convention. * chore(experiments): drop plan references from code comments Plans are ephemeral and get removed when the work is done. Comments that pointed readers at `.plans/experimental-models-1.md` or referenced specific plan sections now stand on their own — the relevant context (sentinel meanings, why the api key column is separate, scope of preview routing) is inlined where it matters instead of forwarded to a doc that won't exist long-term. Touched: - apps/web/src/lib/r2/experiment-prompts.ts (header) - apps/web/src/lib/ai-gateway/experiments/persist.ts (sentinel docs) - apps/web/src/lib/ai-gateway/experiments/upstream-schema.ts (api_key) - apps/web/src/app/admin/model-experiments/ModelExperimentsContent.tsx (admin UI subtitle: dropped trailing 'See .plans/...' sentence) - packages/db/src/schema.ts (three model_experiment* table headers) * chore(experiments): leave shared-with-main files for separate chore PR The previous commit (b6b1ebc) cleaned plan references from five files, three of which already exist on main from earlier merged PRs (Phase 1 schema, Phase 5 admin UI). Touching cross-package files unrelated to the gateway work blew up this PR's diff and risked merge churn against PR #3329, which carries those same three edits independently. Restore the on-main copies of: - packages/db/src/schema.ts - apps/web/src/lib/ai-gateway/experiments/upstream-schema.ts - apps/web/src/app/admin/model-experiments/ModelExperimentsContent.tsx The branch-only files (persist.ts, experiment-prompts.ts) keep their cleanups since they were authored on this branch and land/ship together with the gateway work. * revert(gateway): drop kilo-auto/free experimented-id safeguard Reverts the `filterOutExperimentedPublicIds` filter added in `72dd51171` for `kilo-auto/free` candidate selection. The safeguard added a per-candidate `isPublicIdExperimented` Redis check on every `kilo-auto/free` request — material extra latency on a hot path — to defend against an invariant that holds by construction: `autoFreeModels` and the frontier preset list are hand-curated and never include dedicated preview experiment ids. The explicit-opt-in property of preview models is preserved without any runtime guard. `getProvider`'s experiment-membership check is unchanged — explicit requests to a preview public id still resolve through the experiment routing path. Plan updated to reflect the no-guard decision and rationale. * chore(experiments): drop unused barrel index.ts Every consumer in the repo deep-imports the specific file (`./pick-variant`, `./persist`, `./upstream-schema`, `./build-direct-provider`); nothing imported from the barrel. Most `index.ts` files under `ai-gateway/` are real module implementations, not pure re-exports — drop this one to match. * refactor(experiments): drop system-prompt split, plumb request_kind through capture Adapts the gateway path to the simpler model_experiment_request schema landed in #3358. Addresses comments #2 + #3 on #3325. - ExperimentPromptCapture: remove `systemPromptContent`, add `requestKind`. The full post-`transformRequest` body is now stored as a single content-addressed blob; the upstream API shape is recorded alongside it for downstream reporting. - buildExperimentPromptCapture: collapse the chat_completions-only system-extraction branch; one body capture path for all kinds. - persistExperimentAttribution: one R2 PUT instead of two, one hash written, no `__absent__` sentinel. `PROMPT_HASH_ABSENT` removed; `PROMPT_HASH_FAILED` and `PROMPT_HASH_DELETED` retained. - Tests: drop the system-extraction cases (no longer applicable), add coverage for the three request kinds and a determinism check (content addressing relies on byte-stable serialization). The dedup-on-shared-system-prompt micro-optimization had no measured benefit. If we ever want it back, content-addressing the full body is the better starting point anyway. * refactor(gateway): make balance <= 0 the primary condition on the usage-limit guard Reorders the conjuncts on the usage-limit guard so the salient check reads first and `skipBalanceCheck` is clearly an override flag, not the gating condition. Pure reorder — boolean evaluation is unchanged. Addresses comment #1 on #3325. * fix(experiments): backfill the experimented-public-ids redis cache on miss Previously a Redis miss / corrupt cache / Redis error in `isPublicIdExperimented` fell through to Postgres for a single-id lookup and returned the answer without populating Redis. Result: every request on a cold cache hammered Postgres until something else happened to write the key (admin mutation, etc.). On the cache-miss path we now rebuild the full routing-eligible public id set from `model_experiment` (matching what `recomputeExperimentedPublicIds` writes from the admin router) and SET it back to Redis with the same TTL. Subsequent requests hit Redis again. The backfill is fire-and-forget — if Redis is genuinely down we still return the correct answer for this request. Also drops the per-id-only DB query in favor of the full-set query because the cache contract is the complete list, not per-id presence. Addresses comment #4 on #3325. * refactor(experiments): rename subjectPick → allocationSubject, fix lying 'unreachable' comment Two small clarity fixes from #3325 review: - Rename the local variable bound from `pickAllocationSubject(input)` from `subjectPick` to `allocationSubject`. Matches the field name on the result and answers the reviewer's 'is this the user?' question inline. Pure rename. - Replace the 'Unreachable: getRandomNumber returns [0, totalWeight). Fail closed.' comment with an honest one. The branch is defensive fail-closed insurance for the case where `getRandomNumber`'s contract breaks (NaN, negative, weight rounding), not actually unreachable code. The captureMessage + 'unavailable' return is the right behavior; only the comment was wrong. Addresses comments #5 and #6 on #3325. * fix(gateway): return usage identity for zero-cost inserts * fix(gateway): use JS-side identity values to avoid Postgres timestamp format * refactor(gateway): extract experiment variant types * refactor(gateway): narrow experiment attribution catch * refactor(gateway): move prompt storage fallback * refactor(gateway): simplify prompt hash fallback * refactor(gateway): inline experiment attribution insert * refactor(gateway): remove prompt truncation breadcrumb * refactor(gateway): inline prompt capture serialization * refactor(gateway): simplify experiment attribution guard * fix(web): require BYOK encryption key * refactor(gateway): remove missing BYOK key branch * refactor(gateway): cache experimented ids from Redis only * refactor(gateway): load experiment routing from Postgres * test(gateway): avoid silent Redis membership skips * feat(gateway): expose active experiment models * feat(admin): show experiment request log * fix(gateway): allow anonymous experiment models * refactor(gateway): treat experiment models as free models Resolve isPublicIdExperimented once per request and OR the result into the existing free-model gates: rate limit, anonymous-access, and balance check. Drop the now-redundant skipBalanceCheck flag from GetProviderProviderResult. * chore(gateway): remove unrelated review changes * refactor(gateway): fold experiment check into isFreeModel Now that isFreeModel is async (PR #3393), let it answer for experimented public ids directly. Drops the route's local isExperimentedModel boolean from the auth and balance gates and removes the provider_funded flag — cost zeroing is handled by the isFreeModel check in processUsage.ts. * chore(gateway): revert unnecessary comment edits * fix(gateway): refuse experiments under enterprise allow-list Enterprise provider allow-list is enforced via OpenRouter's body.provider.only field, which doesn't reach a direct partner upstream. Refuse experiment routing rather than silently bypassing the org's allow-list. Note Option 3 (unify direct-upstream routing abstraction) in the plan as the proper followup. * fix(gateway): remove duplicate unfiltered experiment models Experiment models were already added to restrictionCandidates and folded into the org-restrictions filter, then appended again unfiltered at the end. The duplicate append created two issues: catalog entries appeared twice, and Enterprise model_deny_list entries were silently re-exposed. Drop the duplicate append; the filter at line 32-42 covers experiments and matches the gateway's behavior (experiments do not set bypassAccessCheck and go through checkOrganizationModelRestrictions on the request path). * fix(gateway): split membership and isFreeModel out of models.ts models.ts is reachable from client bundles via constants like PRIMARY_DEFAULT_MODEL and preferredModels. Folding the experiment-membership check into isFreeModel inside models.ts dragged in the Redis client (and via pick-variant.ts the Postgres client), breaking the production build with 'Module not found' errors for net/tls/dns/promises and pg. Split along two seams that already existed: pick-variant.ts now contains only the drizzle-using routing (getRoutingExperimentForPublicId, pickModelExperimentVariant), while a new membership.ts holds the Redis-only isPublicIdExperimented. The async isFreeModel itself moves to a new is-free-model.ts that imports the sync helpers from models.ts and the membership check from membership.ts. Net result: models.ts has no I/O dependencies, the fold is preserved for server callers, and the experiment-membership check still lives once in the codebase. * docs(experiments): note response model-id leak as v1 followup

Each of the four CREATE UNIQUE INDEX CONCURRENTLY statements in 0245_calm_lockheed.sql had no idempotency guard. CONCURRENTLY can leave an INVALID index under its name if a build fails partway or is interrupted; a blind retry of CREATE UNIQUE INDEX CONCURRENTLY on the same name then fails with 'already exists' instead of rebuilding it - the exact non-retryable strand Finding #1 addressed, just moved down to the index-build step. Precede each build with DROP INDEX CONCURRENTLY IF EXISTS on its exact name: a no-op on the happy path (index doesn't exist yet), and a clean removal of a leftover invalid index otherwise, so every retry converges to the same end state. Per packages/db/AGENTS.md, deliberately not using CREATE ... CONCURRENTLY IF NOT EXISTS, since that would leave a leftover invalid index in place unrebuilt instead of recovering it; DROP INDEX CONCURRENTLY is the exact recovery mechanism the skill names, just applied proactively rather than only after manual diagnosis. Both DROP CONCURRENTLY and CREATE CONCURRENTLY remain in the existing post-COMMIT/pre-BEGIN section, outside any transaction block, as required. No change to the final index set, names, or schema.ts/snapshots. Verified: drizzle-kit check passes. Full chain applies cleanly from scratch. Directly simulated the failure mode: seeded a duplicate-data conflict, forced one of the four builds to fail and leave an INVALID index (confirmed indisvalid=false), confirmed a blind retry without this guard fails with 'already exists' (reproducing the bug), then repaired the data and confirmed the guarded migration now retries cleanly to a fully valid final index set. packages/db/src/schema.test.ts (103 tests) passes.

* feat(github): enable shared web agent installations * fix(github): harden shared installation races * fix(github): serialize shared callback cleanup * fix(integrations): fence provider state mutations * fix(integrations): serialize provider state sessions * fix(integrations): restore provider session consistency * Revert "fix(integrations): restore provider session consistency" This reverts commit ee85e37. * Revert "fix(integrations): serialize provider state sessions" This reverts commit bbecb73. * Revert "fix(integrations): fence provider state mutations" This reverts commit 25817c7. * feat(integrations): reserve provider OAuth attempts * fix(integrations): lock OAuth reservations by owner row * fix(integrations): fence provider OAuth state purpose * test(integrations): harden OAuth reservation races * test(integrations): prove OAuth owner-row blocking * fix(github): serialize verified owner cardinality * test(integrations): include OAuth state issue time * chore(db): regenerate shared GitHub migration * fix(github): close shared runtime isolation gaps * fix(github): fence exclusive lifecycle retries * fix(github): close legacy authorization edges * fix(github): fence direct token legacy paths * fix(github): retry in-progress lifecycle receipts * refactor(github): simplify lifecycle cleanup * fix(github): align repository refresh lock order * test(github): prove lifecycle lock overlap * test(github): match exact advisory lock query * fix(github): bind bot links to tenant association * fix(github): constrain legacy bot-link fallback * fix(db): reconcile orphans before validating github FK Split 0243 into a pre-flight reconciliation migration and the FK-adding migration: - 0243_reconcile_orphaned_github_installation_id.sql nulls out any platform_integrations.github_installation_id that no longer references an existing github_app_installations row, so the FK added below cannot fail its full-table validation on drift. - 0244_free_thena.sql (renamed from 0243_exotic_triton.sql) now declares the FK with an explicit ON DELETE restrict, matching the repo convention for blocking FKs and the lifecycle already enforced in code: github_app_installations rows are soft-deleted (lifecycle_state/deleted_at), and the only hard-delete path (anonymizeCloudUserData) already guards with a NOT EXISTS check against remaining associations. schema.ts documents the ON DELETE restrict choice inline. * fix(github): only record webhook receipt on 2xx dispatch dispatchSharedOnce recorded a completed delivery receipt whenever the shared-installation receipt lookup wasn't missing_canonical, regardless of the dispatched handler's response status. A non-2xx dispatch was therefore marked completed, so a later GitHub redelivery of the same event was short-circuited as a duplicate and could never retry. Only record the receipt when response.ok, leaving non-2xx dispatches unrecorded so GitHub's redelivery can retry them. missing_canonical behavior is unchanged. * fix(integrations): supersede unexpired pending OAuth attempts beginProviderOAuthAttempt only expired pending/consumed rows whose expires_at was already in the past before inserting the new attempt. An unexpired pending row for the same owner+provider (e.g. an abandoned flow) still collided with the partial unique index on (owner, provider, status = 'pending'), blocking a fresh /connect attempt for up to ATTEMPT_TTL_MS (10 minutes) and surfacing oauth_init_failed. Drop the expires_at filter for pending rows so a new attempt always supersedes an existing pending one for that owner+provider, within the same tx and owner-row lock already held. Consumed rows are still only expired once actually past their TTL, since they don't hold that unique slot. consumeProviderOAuthAttempt's pre-step doesn't insert and matches the row to consume by exact state_hash, so it doesn't share this collision pattern and is unchanged. * fix(cloud-agent): dedupe exact-duplicate repository entries results.flatMap(r => r.repositories) in fetchRepositoriesForIntegrations had no dedup after dedupeRepositories was removed, so a corrupted or duplicated cached repositories array on a single installation would surface the same repo twice for that installation. Deliberately keep one entry per (platformIntegrationId, id) rather than deduping by repo id alone: NewSessionPanel already keys and selects repository rows by platform + platformIntegrationId + id, and the existing 'preserves both association choices when two installations expose one repository' test documents that two different installations granting the same repo must both remain selectable, since session start resolves the token/permissions from the chosen installation. Deduping by repo id would silently drop one of those legitimate, functionally distinct entries. Added a test covering the same-installation duplicate case and confirmed the existing cross-installation test is unaffected. * fix(cloud-agent): repair personal GitHub session start and stale refresh Three must-fix issues found in code review of the GitHub connections work: 1. NewSessionPanel's GitHub-integration guard was unconditional on selectedGitHubIntegrationId, but that field is only ever populated for organization-scoped repos. Every personal (non-org) user selecting a GitHub repo hit 'Select the GitHub repository again' and could never start a session. Scope the guard to organizationId, matching the sibling conditional already used for creationIntent. 2. Both listGitHubRepositories tRPC output schemas silently stripped githubAppType (and the personal router also stripped platformIntegrationId/platformAccountLogin), making the new 'Lite' badge dead code and contributing to (1). Added the missing fields to both schemas. 3. refreshInstallation stopped syncing account/permissions/scopes/ repository_access/installed_at onto the platform_integrations association row after the shared-installation refactor removed the legacy upsertPlatformIntegrationForOwner call. getInstallation/ listIntegrations read those columns directly off platform_integrations, so GitHubIntegrationDetails kept showing stale data after a refresh. Added syncIntegrationInstallationDetails, scoped by integration id only (safe for shared/multi-owner installations, unlike the removed legacy writer), and wired it into refreshInstallation. Added regression tests for all three: both listGitHubRepositories routers now assert a full repo object round-trips unchanged through the real tRPC output validation, and refreshInstallation's tests assert syncIntegrationInstallationDetails is called with fresh data (and not called on the early-exit failure path). * fix(db): make github FK migration crash-safe and guard orphan reconciliation Split 0244 into three independently-tracked migrations so a failed or interrupted deploy can never strand durably-committed DDL behind an untracked migration, and never leaves a window with neither the old nor the new uniqueness protection in place: - 0244_salty_cyclops.sql: plain transactional additions only (new tables, new columns, all FK constraints including the validating platform_integrations FK). No explicit COMMIT/BEGIN, so Postgres rolls the whole file back atomically on any failure; a retry is always clean. - 0245_calm_lockheed.sql: only the four CREATE UNIQUE INDEX CONCURRENTLY replacement indexes, fenced with the repo's approved COMMIT/BEGIN pattern. Does NOT drop the old two unique indexes. - 0246_clean_the_renegades.sql: drops the two old unique indexes and adds the sharing_mode CHECK constraint, only after 0245 has been independently tracked as applied, so the replacements are always confirmed built before the originals are removed. Verified empirically: applying only through 0245 leaves both the old and new indexes coexisting (all valid) - there is no point in the sequence where the uniqueness invariant is unprotected. 0246 then applies cleanly as its own later migration. Per packages/db/AGENTS.md, deliberately did not add IF NOT EXISTS or a pre-emptive DROP INDEX IF EXISTS around the CONCURRENTLY builds: the skill explicitly prohibits automatically masking a failed/invalid concurrent index build that way and requires explicit operator inspection and recovery (DROP INDEX CONCURRENTLY / REINDEX INDEX CONCURRENTLY) instead. The file split already achieves retry-safety at the migration-tracking level without needing that. Also hardens 0243: replace the silent UPDATE that nulled any dangling platform_integrations.github_installation_id with a DO 12662 guard that RAISEs if any orphan exists. Nulling was a behavior change in disguise - for a row with github_app_type IS NULL, clearing the reference makes it newly eligible for the legacy bot-link fallback lookup (which treats github_app_type = 'standard' OR IS NULL as equivalent). Since production orphan_count is confirmed 0, assert the invariant instead of silently mutating data/eligibility. Verified: drizzle-kit check passes; the full chain (including a staged partial-apply through 0245 only, then 0246 separately) applies cleanly from scratch against a fresh local Postgres; schema.ts is unchanged (same end-state schema, just staged across more granular migrations); packages/db/src/schema.test.ts (103 tests) passes. * fix(db): make concurrent index builds in 0245 retry-safe Each of the four CREATE UNIQUE INDEX CONCURRENTLY statements in 0245_calm_lockheed.sql had no idempotency guard. CONCURRENTLY can leave an INVALID index under its name if a build fails partway or is interrupted; a blind retry of CREATE UNIQUE INDEX CONCURRENTLY on the same name then fails with 'already exists' instead of rebuilding it - the exact non-retryable strand Finding #1 addressed, just moved down to the index-build step. Precede each build with DROP INDEX CONCURRENTLY IF EXISTS on its exact name: a no-op on the happy path (index doesn't exist yet), and a clean removal of a leftover invalid index otherwise, so every retry converges to the same end state. Per packages/db/AGENTS.md, deliberately not using CREATE ... CONCURRENTLY IF NOT EXISTS, since that would leave a leftover invalid index in place unrebuilt instead of recovering it; DROP INDEX CONCURRENTLY is the exact recovery mechanism the skill names, just applied proactively rather than only after manual diagnosis. Both DROP CONCURRENTLY and CREATE CONCURRENTLY remain in the existing post-COMMIT/pre-BEGIN section, outside any transaction block, as required. No change to the final index set, names, or schema.ts/snapshots. Verified: drizzle-kit check passes. Full chain applies cleanly from scratch. Directly simulated the failure mode: seeded a duplicate-data conflict, forced one of the four builds to fail and leave an INVALID index (confirmed indisvalid=false), confirmed a blind retry without this guard fails with 'already exists' (reproducing the bug), then repaired the data and confirmed the guarded migration now retries cleanly to a fully valid final index set. packages/db/src/schema.test.ts (103 tests) passes. * fix(github): restore exact-id shared runtime authorization lost in the main merge The main #6178/#6179 reconciliation merge (81628ee) kept sharing_mode gating unconditionally exclusive-only, which regressed a shared (web_cloud_agent) association's ability to ever authorize, even with an exact association id pinned. Note: this carve-out never actually existed on feat/github-connections-pr2 before that merge (verified via git history: it only exists on feat/github-connections-pr4's commits 15ae070 and f53e068) — but it is a real, needed capability for shared installations to read/refresh their own repository inventory, so re-applying it here now, adapted to main's reason-returning shape: 1. getGitHubRuntimeAssociationRejectionReason(association, options?) takes { allowShared?: boolean }; sharing_mode is only permitted to be 'web_cloud_agent' when allowShared is explicitly true, otherwise it must be 'exclusive'. isGitHubRuntimeAssociationAuthorized forwards the same options (defaulting to exclusive-only). 2. assertGitHubInstallationRuntimeAuthorized normalizes expectedIntegrationId once into exactAssociationId (empty string -> undefined) and derives allowShared = exactAssociationId !== undefined, so the id predicate, the row limit, and the sharing-mode carve-out can never diverge (the F3 invariant). 3. The SQL's canonical-installation OR-branch permits sharing_mode IN (exclusive, web_cloud_agent) only when allowShared; otherwise exclusive only. 4. The post-query reason check passes { allowShared } through so it agrees with what the SQL already filtered for. Every other check (ownership, local health, canonical installation health, ambiguous_association for the generic path) is unchanged. Ported the PR4 test coverage for this onto PR2's reason-based API: - runtime-authorization.test.ts: allowShared true/false/unset on the pure reason function and boolean wrapper, plus mock-level exact-id vs generic-path authorization of a shared association. - github-installations.test.ts: a real end-to-end integration test (two orgs sharing one installation) covering exact-id shared authorization, generic-path rejection, an added empty-string-id F3 case, unknown-id rejection, and local/owner health enforcement on the exact-id path; plus canonical-installation-unhealthy rejection. - New review-shared-repositories.test.ts (ported from PR4): an end-to-end adapter-level regression proving fetchGitHubRepositories succeeds for an exact healthy shared association. Did not port PR4's separate connection-service.ts repository-caching- at-attach fix (15ae070) — that is an unrelated bug in a different function, out of scope for this regression fix. Verified: pnpm --filter web typecheck clean; oxlint clean on all 4 changed files; pnpm format (line-wrap only); git diff --check clean; no schema.ts or migration changes. 182 tests pass across the full required suite (runtime-authorization, review-shared-repositories, webhook-handler + webhook-handler-dispatch.integration, installation-handler, connection-service, github-integration-helpers, prepare-review-payload, github-installations).

* feat(github): enable shared web agent installations * fix(github): harden shared installation races * fix(github): serialize shared callback cleanup * fix(integrations): fence provider state mutations * fix(integrations): serialize provider state sessions * fix(integrations): restore provider session consistency * Revert "fix(integrations): restore provider session consistency" This reverts commit ee85e37. * Revert "fix(integrations): serialize provider state sessions" This reverts commit bbecb73. * Revert "fix(integrations): fence provider state mutations" This reverts commit 25817c7. * feat(integrations): reserve provider OAuth attempts * fix(integrations): lock OAuth reservations by owner row * fix(integrations): fence provider OAuth state purpose * test(integrations): harden OAuth reservation races * test(integrations): prove OAuth owner-row blocking * fix(github): serialize verified owner cardinality * test(integrations): include OAuth state issue time * chore(db): regenerate shared GitHub migration * fix(github): close shared runtime isolation gaps * fix(github): fence exclusive lifecycle retries * fix(github): close legacy authorization edges * fix(github): fence direct token legacy paths * fix(github): retry in-progress lifecycle receipts * refactor(github): simplify lifecycle cleanup * fix(github): align repository refresh lock order * test(github): prove lifecycle lock overlap * test(github): match exact advisory lock query * fix(github): bind bot links to tenant association * fix(github): constrain legacy bot-link fallback * feat(slack): fence shared installation activation * feat(slack): delegate with exact GitHub connection * fix(slack): make installation activation recoverable * fix(slack): resolve all-access GitHub repositories * fix(slack): retire expired installation generations * test(slack): reject unhealthy all-access GitHub links * fix(slack): preserve cleanup reservations until revoke * fix(slack): clean verified legacy token metadata * fix(slack): retain cleanup identity barriers * fix(slack): bridge Enterprise workspace identities * fix(slack): order Enterprise identity aliases * fix(slack): fence alias and cleanup ordering * fix(slack): persist cleanup and alias progress * fix(slack): defer shared Enterprise Grid support * fix(slack): isolate shared workspace activation * fix(slack): fence workspace sharing conversion * fix(slack): validate workspace conversion state * fix(slack): route shared and cleanup states exactly * fix(slack): preserve unshared runtime boundaries * fix(slack): enforce runtime health boundaries * fix(slack): reserve successful unshared workspaces * fix(slack): finalize SDK effects with generation CAS * fix(slack): separate workspace and Enterprise cleanup * fix(slack): terminalize legacy cleanup safely * fix(slack): release terminal cleanup tombstones * fix(slack): isolate webhook maintenance failures * fix(slack): exclude Enterprise installs from backfill * fix(slack): validate Enterprise IDs and reset health * fix(slack): make Enterprise activation success-only * fix(slack): reconstruct Enterprise runtime state * Revert "fix(slack): reconstruct Enterprise runtime state" This reverts commit 0d98e23. * Revert "fix(slack): make Enterprise activation success-only" This reverts commit 0440374. * Revert "fix(slack): validate Enterprise IDs and reset health" This reverts commit 06f0792. * Revert "fix(slack): exclude Enterprise installs from backfill" This reverts commit bf4f256. * Revert "fix(slack): isolate webhook maintenance failures" This reverts commit 9bfcd8d. * Revert "fix(slack): release terminal cleanup tombstones" This reverts commit 6d67a1e. * Revert "fix(slack): terminalize legacy cleanup safely" This reverts commit bfe544e. * Revert "fix(slack): separate workspace and Enterprise cleanup" This reverts commit 8ac1656. * Revert "fix(slack): finalize SDK effects with generation CAS" This reverts commit 31e4964. * Revert "fix(slack): reserve successful unshared workspaces" This reverts commit d8eecc6. * Revert "fix(slack): enforce runtime health boundaries" This reverts commit fd6c560. * Revert "fix(slack): preserve unshared runtime boundaries" This reverts commit f386ae0. * Revert "fix(slack): route shared and cleanup states exactly" This reverts commit 185270d. * Revert "fix(slack): validate workspace conversion state" This reverts commit b567d09. * Revert "fix(slack): fence workspace sharing conversion" This reverts commit 3bf72f0. * Revert "fix(slack): isolate shared workspace activation" This reverts commit 31ac813. * Revert "fix(slack): defer shared Enterprise Grid support" This reverts commit 4eab6e0. * Revert "fix(slack): persist cleanup and alias progress" This reverts commit 89ac167. * Revert "fix(slack): fence alias and cleanup ordering" This reverts commit 41538c3. * Revert "fix(slack): order Enterprise identity aliases" This reverts commit 439013f. * Revert "fix(slack): bridge Enterprise workspace identities" This reverts commit 8db09b0. * Revert "fix(slack): retain cleanup identity barriers" This reverts commit 4357a03. * Revert "fix(slack): clean verified legacy token metadata" This reverts commit 0636b8f. * Revert "fix(slack): preserve cleanup reservations until revoke" This reverts commit 1d9b194. * Revert "test(slack): reject unhealthy all-access GitHub links" This reverts commit 296df67. * Revert "fix(slack): retire expired installation generations" This reverts commit 300d02a. * Revert "fix(slack): resolve all-access GitHub repositories" This reverts commit 27d5944. * Revert "fix(slack): make installation activation recoverable" This reverts commit b9aeab1. * Revert "feat(slack): fence shared installation activation" This reverts commit 57f5e26. * test(slack): verify minimal shared delegation * test(slack): preserve unsupported provider fences * test(slack): tighten minimal sharing fixtures * fix(slack): validate cached GitHub repositories * fix(db): reconcile orphans before validating github FK Split 0243 into a pre-flight reconciliation migration and the FK-adding migration: - 0243_reconcile_orphaned_github_installation_id.sql nulls out any platform_integrations.github_installation_id that no longer references an existing github_app_installations row, so the FK added below cannot fail its full-table validation on drift. - 0244_free_thena.sql (renamed from 0243_exotic_triton.sql) now declares the FK with an explicit ON DELETE restrict, matching the repo convention for blocking FKs and the lifecycle already enforced in code: github_app_installations rows are soft-deleted (lifecycle_state/deleted_at), and the only hard-delete path (anonymizeCloudUserData) already guards with a NOT EXISTS check against remaining associations. schema.ts documents the ON DELETE restrict choice inline. * fix(github): only record webhook receipt on 2xx dispatch dispatchSharedOnce recorded a completed delivery receipt whenever the shared-installation receipt lookup wasn't missing_canonical, regardless of the dispatched handler's response status. A non-2xx dispatch was therefore marked completed, so a later GitHub redelivery of the same event was short-circuited as a duplicate and could never retry. Only record the receipt when response.ok, leaving non-2xx dispatches unrecorded so GitHub's redelivery can retry them. missing_canonical behavior is unchanged. * fix(integrations): supersede unexpired pending OAuth attempts beginProviderOAuthAttempt only expired pending/consumed rows whose expires_at was already in the past before inserting the new attempt. An unexpired pending row for the same owner+provider (e.g. an abandoned flow) still collided with the partial unique index on (owner, provider, status = 'pending'), blocking a fresh /connect attempt for up to ATTEMPT_TTL_MS (10 minutes) and surfacing oauth_init_failed. Drop the expires_at filter for pending rows so a new attempt always supersedes an existing pending one for that owner+provider, within the same tx and owner-row lock already held. Consumed rows are still only expired once actually past their TTL, since they don't hold that unique slot. consumeProviderOAuthAttempt's pre-step doesn't insert and matches the row to consume by exact state_hash, so it doesn't share this collision pattern and is unchanged. * fix(cloud-agent): dedupe exact-duplicate repository entries results.flatMap(r => r.repositories) in fetchRepositoriesForIntegrations had no dedup after dedupeRepositories was removed, so a corrupted or duplicated cached repositories array on a single installation would surface the same repo twice for that installation. Deliberately keep one entry per (platformIntegrationId, id) rather than deduping by repo id alone: NewSessionPanel already keys and selects repository rows by platform + platformIntegrationId + id, and the existing 'preserves both association choices when two installations expose one repository' test documents that two different installations granting the same repo must both remain selectable, since session start resolves the token/permissions from the chosen installation. Deduping by repo id would silently drop one of those legitimate, functionally distinct entries. Added a test covering the same-installation duplicate case and confirmed the existing cross-installation test is unaffected. * fix(slack): accept adapter-written repository cache * fix(github): harden shared delivery and repository validation * fix(cloud-agent): repair personal GitHub session start and stale refresh Three must-fix issues found in code review of the GitHub connections work: 1. NewSessionPanel's GitHub-integration guard was unconditional on selectedGitHubIntegrationId, but that field is only ever populated for organization-scoped repos. Every personal (non-org) user selecting a GitHub repo hit 'Select the GitHub repository again' and could never start a session. Scope the guard to organizationId, matching the sibling conditional already used for creationIntent. 2. Both listGitHubRepositories tRPC output schemas silently stripped githubAppType (and the personal router also stripped platformIntegrationId/platformAccountLogin), making the new 'Lite' badge dead code and contributing to (1). Added the missing fields to both schemas. 3. refreshInstallation stopped syncing account/permissions/scopes/ repository_access/installed_at onto the platform_integrations association row after the shared-installation refactor removed the legacy upsertPlatformIntegrationForOwner call. getInstallation/ listIntegrations read those columns directly off platform_integrations, so GitHubIntegrationDetails kept showing stale data after a refresh. Added syncIntegrationInstallationDetails, scoped by integration id only (safe for shared/multi-owner installations, unlike the removed legacy writer), and wired it into refreshInstallation. Added regression tests for all three: both listGitHubRepositories routers now assert a full repo object round-trips unchanged through the real tRPC output validation, and refreshInstallation's tests assert syncIntegrationInstallationDetails is called with fresh data (and not called on the early-exit failure path). * fix(github): reclaim stale installation delivery claims * fix(github): enforce webhook delivery-claim window * fix(db): make github FK migration crash-safe and guard orphan reconciliation Split 0244 into three independently-tracked migrations so a failed or interrupted deploy can never strand durably-committed DDL behind an untracked migration, and never leaves a window with neither the old nor the new uniqueness protection in place: - 0244_salty_cyclops.sql: plain transactional additions only (new tables, new columns, all FK constraints including the validating platform_integrations FK). No explicit COMMIT/BEGIN, so Postgres rolls the whole file back atomically on any failure; a retry is always clean. - 0245_calm_lockheed.sql: only the four CREATE UNIQUE INDEX CONCURRENTLY replacement indexes, fenced with the repo's approved COMMIT/BEGIN pattern. Does NOT drop the old two unique indexes. - 0246_clean_the_renegades.sql: drops the two old unique indexes and adds the sharing_mode CHECK constraint, only after 0245 has been independently tracked as applied, so the replacements are always confirmed built before the originals are removed. Verified empirically: applying only through 0245 leaves both the old and new indexes coexisting (all valid) - there is no point in the sequence where the uniqueness invariant is unprotected. 0246 then applies cleanly as its own later migration. Per packages/db/AGENTS.md, deliberately did not add IF NOT EXISTS or a pre-emptive DROP INDEX IF EXISTS around the CONCURRENTLY builds: the skill explicitly prohibits automatically masking a failed/invalid concurrent index build that way and requires explicit operator inspection and recovery (DROP INDEX CONCURRENTLY / REINDEX INDEX CONCURRENTLY) instead. The file split already achieves retry-safety at the migration-tracking level without needing that. Also hardens 0243: replace the silent UPDATE that nulled any dangling platform_integrations.github_installation_id with a DO 12662 guard that RAISEs if any orphan exists. Nulling was a behavior change in disguise - for a row with github_app_type IS NULL, clearing the reference makes it newly eligible for the legacy bot-link fallback lookup (which treats github_app_type = 'standard' OR IS NULL as equivalent). Since production orphan_count is confirmed 0, assert the invariant instead of silently mutating data/eligibility. Verified: drizzle-kit check passes; the full chain (including a staged partial-apply through 0245 only, then 0246 separately) applies cleanly from scratch against a fresh local Postgres; schema.ts is unchanged (same end-state schema, just staged across more granular migrations); packages/db/src/schema.test.ts (103 tests) passes. * fix(db): make concurrent index builds in 0245 retry-safe Each of the four CREATE UNIQUE INDEX CONCURRENTLY statements in 0245_calm_lockheed.sql had no idempotency guard. CONCURRENTLY can leave an INVALID index under its name if a build fails partway or is interrupted; a blind retry of CREATE UNIQUE INDEX CONCURRENTLY on the same name then fails with 'already exists' instead of rebuilding it - the exact non-retryable strand Finding #1 addressed, just moved down to the index-build step. Precede each build with DROP INDEX CONCURRENTLY IF EXISTS on its exact name: a no-op on the happy path (index doesn't exist yet), and a clean removal of a leftover invalid index otherwise, so every retry converges to the same end state. Per packages/db/AGENTS.md, deliberately not using CREATE ... CONCURRENTLY IF NOT EXISTS, since that would leave a leftover invalid index in place unrebuilt instead of recovering it; DROP INDEX CONCURRENTLY is the exact recovery mechanism the skill names, just applied proactively rather than only after manual diagnosis. Both DROP CONCURRENTLY and CREATE CONCURRENTLY remain in the existing post-COMMIT/pre-BEGIN section, outside any transaction block, as required. No change to the final index set, names, or schema.ts/snapshots. Verified: drizzle-kit check passes. Full chain applies cleanly from scratch. Directly simulated the failure mode: seeded a duplicate-data conflict, forced one of the four builds to fail and leave an INVALID index (confirmed indisvalid=false), confirmed a blind retry without this guard fails with 'already exists' (reproducing the bug), then repaired the data and confirmed the guarded migration now retries cleanly to a fully valid final index set. packages/db/src/schema.test.ts (103 tests) passes. * fix(github): restore exact-id shared runtime authorization lost in the main merge The main #6178/#6179 reconciliation merge (81628ee) kept sharing_mode gating unconditionally exclusive-only, which regressed a shared (web_cloud_agent) association's ability to ever authorize, even with an exact association id pinned. Note: this carve-out never actually existed on feat/github-connections-pr2 before that merge (verified via git history: it only exists on feat/github-connections-pr4's commits 15ae070 and f53e068) — but it is a real, needed capability for shared installations to read/refresh their own repository inventory, so re-applying it here now, adapted to main's reason-returning shape: 1. getGitHubRuntimeAssociationRejectionReason(association, options?) takes { allowShared?: boolean }; sharing_mode is only permitted to be 'web_cloud_agent' when allowShared is explicitly true, otherwise it must be 'exclusive'. isGitHubRuntimeAssociationAuthorized forwards the same options (defaulting to exclusive-only). 2. assertGitHubInstallationRuntimeAuthorized normalizes expectedIntegrationId once into exactAssociationId (empty string -> undefined) and derives allowShared = exactAssociationId !== undefined, so the id predicate, the row limit, and the sharing-mode carve-out can never diverge (the F3 invariant). 3. The SQL's canonical-installation OR-branch permits sharing_mode IN (exclusive, web_cloud_agent) only when allowShared; otherwise exclusive only. 4. The post-query reason check passes { allowShared } through so it agrees with what the SQL already filtered for. Every other check (ownership, local health, canonical installation health, ambiguous_association for the generic path) is unchanged. Ported the PR4 test coverage for this onto PR2's reason-based API: - runtime-authorization.test.ts: allowShared true/false/unset on the pure reason function and boolean wrapper, plus mock-level exact-id vs generic-path authorization of a shared association. - github-installations.test.ts: a real end-to-end integration test (two orgs sharing one installation) covering exact-id shared authorization, generic-path rejection, an added empty-string-id F3 case, unknown-id rejection, and local/owner health enforcement on the exact-id path; plus canonical-installation-unhealthy rejection. - New review-shared-repositories.test.ts (ported from PR4): an end-to-end adapter-level regression proving fetchGitHubRepositories succeeds for an exact healthy shared association. Did not port PR4's separate connection-service.ts repository-caching- at-attach fix (15ae070) — that is an unrelated bug in a different function, out of scope for this regression fix. Verified: pnpm --filter web typecheck clean; oxlint clean on all 4 changed files; pnpm format (line-wrap only); git diff --check clean; no schema.ts or migration changes. 182 tests pass across the full required suite (runtime-authorization, review-shared-repositories, webhook-handler + webhook-handler-dispatch.integration, installation-handler, connection-service, github-integration-helpers, prepare-review-payload, github-installations).

…testing (#6100) * feat(github): enable shared web agent installations * fix(github): harden shared installation races * fix(github): serialize shared callback cleanup * fix(integrations): fence provider state mutations * fix(integrations): serialize provider state sessions * fix(integrations): restore provider session consistency * Revert "fix(integrations): restore provider session consistency" This reverts commit ee85e37126e6cb898da4033490f53fb03526e586. * Revert "fix(integrations): serialize provider state sessions" This reverts commit bbecb7325122c5d77047992074a86f250f79e8a3. * Revert "fix(integrations): fence provider state mutations" This reverts commit 25817c7c03bedfd2226ac430668f56492c4ab19b. * feat(integrations): reserve provider OAuth attempts * fix(integrations): lock OAuth reservations by owner row * fix(integrations): fence provider OAuth state purpose * test(integrations): harden OAuth reservation races * test(integrations): prove OAuth owner-row blocking * fix(github): serialize verified owner cardinality * test(integrations): include OAuth state issue time * chore(db): regenerate shared GitHub migration * fix(github): close shared runtime isolation gaps * fix(github): fence exclusive lifecycle retries * fix(github): close legacy authorization edges * fix(github): fence direct token legacy paths * fix(github): retry in-progress lifecycle receipts * refactor(github): simplify lifecycle cleanup * fix(github): align repository refresh lock order * test(github): prove lifecycle lock overlap * test(github): match exact advisory lock query * fix(github): bind bot links to tenant association * fix(github): constrain legacy bot-link fallback * feat(slack): fence shared installation activation * feat(slack): delegate with exact GitHub connection * fix(slack): make installation activation recoverable * fix(slack): resolve all-access GitHub repositories * fix(slack): retire expired installation generations * test(slack): reject unhealthy all-access GitHub links * fix(slack): preserve cleanup reservations until revoke * fix(slack): clean verified legacy token metadata * fix(slack): retain cleanup identity barriers * fix(slack): bridge Enterprise workspace identities * fix(slack): order Enterprise identity aliases * fix(slack): fence alias and cleanup ordering * fix(slack): persist cleanup and alias progress * fix(slack): defer shared Enterprise Grid support * fix(slack): isolate shared workspace activation * fix(slack): fence workspace sharing conversion * fix(slack): validate workspace conversion state * fix(slack): route shared and cleanup states exactly * fix(slack): preserve unshared runtime boundaries * fix(slack): enforce runtime health boundaries * fix(slack): reserve successful unshared workspaces * fix(slack): finalize SDK effects with generation CAS * fix(slack): separate workspace and Enterprise cleanup * fix(slack): terminalize legacy cleanup safely * fix(slack): release terminal cleanup tombstones * fix(slack): isolate webhook maintenance failures * fix(slack): exclude Enterprise installs from backfill * fix(slack): validate Enterprise IDs and reset health * fix(slack): make Enterprise activation success-only * fix(slack): reconstruct Enterprise runtime state * Revert "fix(slack): reconstruct Enterprise runtime state" This reverts commit 0d98e23901e2bf5f0eab1808cd933d8d04f4a44d. * Revert "fix(slack): make Enterprise activation success-only" This reverts commit 044037412d82dfacf43ae6096eb4a6b11d9f3e58. * Revert "fix(slack): validate Enterprise IDs and reset health" This reverts commit 06f0792ade52271eb9787f5b4379cb9e760caea9. * Revert "fix(slack): exclude Enterprise installs from backfill" This reverts commit bf4f2569591b9849677439d7bb23d0dd174d3d5e. * Revert "fix(slack): isolate webhook maintenance failures" This reverts commit 9bfcd8d922f5db7670f2e1335e5714a9346151b7. * Revert "fix(slack): release terminal cleanup tombstones" This reverts commit 6d67a1ee2e6e0f22bdd0b6fa7789dccc04aa50f3. * Revert "fix(slack): terminalize legacy cleanup safely" This reverts commit bfe544e479c5ffbe394c6b54b46d2b08d1062e70. * Revert "fix(slack): separate workspace and Enterprise cleanup" This reverts commit 8ac165699e3851a48df175bfd299739453645e1d. * Revert "fix(slack): finalize SDK effects with generation CAS" This reverts commit 31e4964945b55982399415304f8f090534d5d9a2. * Revert "fix(slack): reserve successful unshared workspaces" This reverts commit d8eecc65f677b302d373faca1a4a1343bc341e51. * Revert "fix(slack): enforce runtime health boundaries" This reverts commit fd6c560f7bdbb72afb4c98d9cceb3ad5fd3750b1. * Revert "fix(slack): preserve unshared runtime boundaries" This reverts commit f386ae078fb09435d612a65f21e2893b0852f8dc. * Revert "fix(slack): route shared and cleanup states exactly" This reverts commit 185270d903eb3a9621908e15f8db0be7e42c2c37. * Revert "fix(slack): validate workspace conversion state" This reverts commit b567d09d9d3a4a082ff2c4e1438986f986d8a524. * Revert "fix(slack): fence workspace sharing conversion" This reverts commit 3bf72f03b2a7c23d07981720a6d35ddfb9a0ce3f. * Revert "fix(slack): isolate shared workspace activation" This reverts commit 31ac81376339c423e256696f47d57a5cf4d5c44b. * Revert "fix(slack): defer shared Enterprise Grid support" This reverts commit 4eab6e07537054d35481c7a6798f0bd144fb6a59. * Revert "fix(slack): persist cleanup and alias progress" This reverts commit 89ac1672a8382e8daaf1bb40fa3d6fa2367594bd. * Revert "fix(slack): fence alias and cleanup ordering" This reverts commit 41538c3b411ca91530bd40bcd19a094fc2a61df1. * Revert "fix(slack): order Enterprise identity aliases" This reverts commit 439013f6fbac6eea73befddbabdf8fbd5e265b8b. * Revert "fix(slack): bridge Enterprise workspace identities" This reverts commit 8db09b0d2908043e9322da6dc86a2ffad7d65996. * Revert "fix(slack): retain cleanup identity barriers" This reverts commit 4357a03a2d58770443ccbff3e08a9771e65a20ed. * Revert "fix(slack): clean verified legacy token metadata" This reverts commit 0636b8f71bc85fec094d18bf26a621d299f049b0. * Revert "fix(slack): preserve cleanup reservations until revoke" This reverts commit 1d9b19497717dc08e67d95376c07a83e70fbe1b6. * Revert "test(slack): reject unhealthy all-access GitHub links" This reverts commit 296df672a973e6d576df7c138c979eaae1c32c34. * Revert "fix(slack): retire expired installation generations" This reverts commit 300d02a4d1069b0579518b120b8f9831fb772a54. * Revert "fix(slack): resolve all-access GitHub repositories" This reverts commit 27d5944fa715d0008018ee64a7723b5260603bb5. * Revert "fix(slack): make installation activation recoverable" This reverts commit b9aeab17f9b307f257bc555f994d09fc3ae30a56. * Revert "feat(slack): fence shared installation activation" This reverts commit 57f5e26032452695fa9a41d9d8ab8d67f54159dd. * test(slack): verify minimal shared delegation * test(slack): preserve unsupported provider fences * test(slack): tighten minimal sharing fixtures * fix(slack): validate cached GitHub repositories * fix(github): stop a locally disconnected connection from blocking new ones After local disconnect, the platform_integrations row stayed included in every "does this org already have an installation" cardinality check, permanently occupying the org's single-installation slot and hiding both the fresh-install and connect-existing entry points. It also could never be hard-uninstalled afterward, because uninstallExclusiveGitHubInstallation required the target association itself to still be locally connected. - Exclude locally disconnected rows from the fresh-install (mintInstallState) and connect-existing (listOrganizationInstallations canAdd/ existingConnectionAdmission) cardinality checks, and from connectVerifiedGitHubInstallation's owner-cardinality guard. - Fix uninstallExclusiveGitHubInstallation to key off whether any OTHER tenant association is still connected, not whether the target association itself is; a disconnected sole association can now be hard-uninstalled. - Update the settings dropdown to offer real removal (upstream uninstall) instead of a no-op repeat "Disconnect" once a connection is already disconnected locally. No new lifecycle machine; reuses the existing disconnect/uninstall writers and their advisory-lock ordering. * fix(organizations): keep onboarding Source Control in sync with GitHub getOrganizationOnboardingState already used the same health predicate as the integration list (platformIntegrationHealthSql), so the reported '0/3 Source Control incomplete despite an active connection' symptom traced to a UI staleness gap rather than the status source itself: a GitHub connection made from the organization's Settings/Integrations page (fresh install or Connect existing) has no way to notify an onboarding wizard tab left open from before that change. Add the same window-focus/visibilitychange refetch the wizard's own embedded GitHub setup UI already uses for its returning-tab case, applied to the checklist query generally, so Source Control reflects the current connection state when the tab regains focus. Add DB-level regression coverage locking in that the checklist already correctly reflects: an active connection created through the canonical connectVerifiedGitHubInstallation writer, its clearing on local disconnect, and suspended/auth-invalid/disconnected/pending GitHub integrations as incomplete (previously untested negative paths). * fix(github): record authorization provenance on every connect path github_authorized_by_user_id/github_authorized_user_id/github_authorized_at were only ever set by connectVerifiedGitHubInstallation, leaving them null on two other paths that do have an available identity to record: - The main install callback's legacy branch (connection management disabled, or the weaker access-list assertUserAdministersInstallation check) still exchanges a real authenticated GitHub identity via exchangeGitHubOAuthCode; it just never carried that identity, or the signed-in Kilo user, into upsertPlatformIntegrationForOwner. - autoCompleteInstallation, which finishes a pending admin-approval request from the installation.created webhook, had the original requester's already-OAuth-verified Kilo and GitHub identity sitting on the pending row's metadata but never copied it into the provenance columns. Extend upsertPlatformIntegrationForOwner with optional kiloUserId/ githubUserId, thread the callback's exchanged identity through, and have autoCompleteInstallation record the requester's identity as the authorizer. Documented in code: the pending-approval path can misattribute authorship if a different GitHub org admin approves someone else's request, since no stronger identity is available at webhook time; this is the best available provenance on that path, not a fully independent verification. * fix(github): also exclude disconnected rows from the legacy cardinality guard connectVerifiedGitHubInstallation's owner-cardinality guard was fixed to ignore locally disconnected rows, but upsertPlatformIntegrationForOwner's equivalent guard (the legacy writer used when connection management is disabled) still counted them, so a disconnected standard installation kept blocking a fresh install with multiple_installations_disabled even though the UI/API gate reported the slot as available. Add the same isNull(github_disconnected_at) condition to the legacy writer's ownerIntegrations query, and add a DB-backed regression test: legacy connect -> blocked second install -> local disconnect -> fresh install now succeeds. * fix(github): terminalize active review work on disconnect and uninstall Disconnect and exclusive uninstall never touched the association's own review work. A queued/running review kept its dispatch reservation and non-terminal status after disconnect, and uninstallExclusiveGitHubInstallation deleted the association out from under it (platform_integration_id is ON DELETE SET NULL, not cascade), orphaning it forever. Worse, evaluateGitHubSharingCompatibility inspects every canonical association including disconnected ones, so that stale active review then rejected a later connect-existing attempt from a different, legitimately approved organization with incompatible_workflow. - Generalize cancelActiveCodeReviewsForIntegration (previously org-only, with zero production callers) to take an Owner so it also covers user-owned integrations, accept an optional transaction so it can run under disconnect/uninstall's own advisory-lock ordering, and null dispatch_reservation_id on cancel to match the terminalization convention already used by the stale-review reaper. - Call it from disconnectGitHubInstallation and uninstallExclusiveGitHubInstallation, before the delete in the uninstall case. - Defense in depth: exclude locally disconnected associations from evaluateGitHubSharingCompatibility's association set entirely, so a disconnected tenant's other stale resources (automation, bot requests, deployments) also stop counting as incumbent work. - Update disableBitbucketCodeReviewerForIntegration, the one existing caller, for the generalized signature. DB tests: dispatch reservation clearing, user-owned integration coverage, and an end-to-end disconnect scenario asserting the association's own review/attempt reach a terminal state, an unrelated integration's review is untouched, and a newly approved organization can then connect-existing to the same canonical installation. * test(github): add remaining P2 coverage from PR4 review - connection-service.test: assert a completed connection attempt records authorization provenance, and add an end-to-end create-attempt -> disconnect -> new-attempt -> complete flow proving connection-attempt completion succeeds after a prior local disconnect. - callback route test: assert that a management-enabled install routes through connectVerifiedGitHubInstallation with both the Kilo user id and the verified GitHub user id, not just the legacy writer. - devAddInstallation (dev-only E2E seeding shortcut): document in code why it intentionally omits authorization provenance (no live OAuth identity exists on this path), and add tests asserting it is rejected outside NODE_ENV=development and that it does not pass kiloUserId/ githubUserId when it does run. * fix(github): settle the review ledger only after disconnect/uninstall commits cancelActiveCodeReviewsForIntegration settled the operation ledger through the default db connection immediately after the cancellation query, even when the caller supplied its own transaction. If that caller transaction later rolled back (for example, uninstallExclusiveGitHubInstallation's deleteUpstream throwing), the review cancellation itself was undone but the ledger had already gone terminal (no_op) on a separate connection, leaving the ledger/outbox permanently out of sync with the review's real state. Follow the pattern already used by disableBitbucketCodeReviewerForIntegration: when a transaction is supplied, skip settling inside cancelActiveCodeReviewsForIntegration and let the caller settle (settleCancelledReviews, now exported) only after its own transaction has committed. disconnectGitHubInstallation and uninstallExclusiveGitHubInstallation now return the cancelled rows from their transaction and settle right after. Standalone callers without a transaction keep the exact same best-effort, immediate-settle behavior as before. DB tests: a successful uninstall with an active review/attempt/reservation terminalizes both, clears the reservation, settles the ledger to no_op, and the FK's ON DELETE SET NULL leaves the now-unlinked review terminal; a deleteUpstream that throws rolls back the cancellation, the attempt, and the association delete together, and never settles the ledger (status stays admitted, settled_at stays null). * feat(github): converge Connect GitHub and Connect existing into one flow A user has no way to know ahead of time whether the Kilo GitHub App is already installed somewhere they can access, so they had no way to know whether to click 'Connect GitHub'/'Add organization' (fresh install, skips OAuth) or 'Connect existing' (OAuth-first, discover, pick) — two separate top-level buttons for what is really one intent. Converge on a single primary action. When connection management is enabled, it always begins the existing purpose-bound OAuth attempt (unchanged beginConnection); the resulting picker (GitHubConnectionAttemptState) now shows both the discovered attachable installations AND an explicit 'install on a different GitHub organization' option side by side, so a first-time org still gets a clear path to install even with zero discovered candidates. Selecting an installation still goes through the exact same connect-existing confirm path (selectConnectionInstallation -> connectVerifiedGitHubInstallation); install-new still goes through the exact same mint+redirect path (mintInstallState). Neither path, nor its authorization, changes. When connection management is disabled, the primary action still calls the direct-to-installer mint flow unchanged: beginConnection is gated behind connection management and would reject an OAuth-first attempt in that mode, so today's behavior is preserved exactly. Hide the top-level primary action while a connection attempt's picker is showing, so the surface has exactly one primary decision point at a time. Extend GitHubConnectionAttemptState's existing render tests for the new install-new option in both the empty and non-empty candidate states, and gated by whether a fresh install is currently permitted. * fix(github): distinguish a stale connection attempt from a transient error GitHubConnectionAttemptState always claimed the connection attempt 'expired or is no longer available' on any query error, including a transient network failure or server error where the attempt itself may still be perfectly valid. Branch on the tRPC error code: only a confirmed NOT_FOUND shows the expired/restart copy and the full-restart action; any other error shows a neutral 'could not load' message with a retry that just refetches the same attempt instead of forcing a brand new OAuth round trip. Also give each candidate button in the picker an explicit accessible name (its bare visible text was only the account login, with the raw installation ID as a second, unlabeled text node): 'Connect {login}, installation {id}', with the raw ID marked aria-hidden so it isn't duplicated in the accessible name. No authorization, router, ledger, or data-model change. * fix(github): actually send code_verifier through the OAuth token exchange Live two-org demo testing found the connect/attach flow (and by extension the unified picker's discover stage) failing with connection_failed. GitHub's token endpoint was rejecting the exchange with invalid_grant: 'A code_verifier was not included, but the authorization request included a code_challenge. Codes requested with a code_challenge MUST be redeemed with a code_verifier.' Root cause: @octokit/oauth-methods' exchangeWebFlowCode never forwards a supplied codeVerifier to GitHub for clientType: 'github-app' — confirmed against the installed package (@octokit/oauth-methods@6.0.2): there is no code_verifier/codeVerifier reference anywhere in it. beginConnection requests a PKCE-bound code (code_challenge/code_challenge_method=S256) and connection-state stores the matching codeVerifier; the connection callback correctly passed it into exchangeGitHubOAuthCode, but that verifier was then silently dropped before reaching GitHub. Exchange directly with GitHub's token endpoint instead, mirroring exchangeGitHubUserAuthorizationCode in user-authorization.ts, which already does this correctly and was unaffected (it never went through exchangeWebFlowCode). Also surface GitHub's actual error body (error/error_description) in the thrown error instead of a generic message — that's what let this get diagnosed. Non-PKCE callers (the install callback's setup_action=request/install admin-proof paths, which call exchangeGitHubOAuthCode with no codeVerifier) are unaffected: the field is simply omitted from the request when absent, as it always was. Added apps/web/src/lib/integrations/platforms/github/adapter.test.ts (new file — no prior unit tests existed for this adapter): asserts the outbound request includes code_verifier when provided and omits it when not, and that GitHub's error body is surfaced. A full OAuth round trip isn't unit-testable; this request-shape assertion is what prevents this exact regression. * fix(github): do not require sharing admission for a disconnected prior tenant connectVerifiedGitHubInstallation's otherOwnerAssociations query was not filtered by github_disconnected_at, unlike the equivalent guards introduced elsewhere in this stack (sharing-compatibility.ts, platform-integrations.ts, github-apps-router.ts). A fully disconnected prior tenant's stale row still forced requiresSharingAdmission=true for a new tenant, requiring that new tenant to be on the shared-installation allowlist even though there is no actual concurrent tenant anymore. Split the check: a personal (non-org) owner is still rejected unconditionally whenever *any* other owner has ever claimed this installation, connected or not (claimed_by_other_owner) — that protection must not weaken based on the other owner's connection state. Only the *sharing-admission* requirement (allowlist + compatibility check) is now scoped to *actively connected* other-owner associations, since a disconnected one is no longer real concurrent incumbent work. DB test: a new, non-allowlisted organization can attach to an installation as an ordinary (non-shared) first tenant once the prior owner has disconnected, and the canonical row's sharing_mode stays 'exclusive' (proving the admission path was skipped, not merely approved). * refactor(github): reduce duplication in provenance and settle-contract docs - Extract buildGitHubAuthorizationProvenance, shared by autoCompleteInstallation and upsertPlatformIntegrationForOwner, which previously built an identical-shaped provenance object independently in each. - Name the inline owner: { type: org|user; id } shape on IntegrationReviewCancellationInput as ReviewOwnerRef, so it reads as distinct from this same file's own (differently-shaped) Owner and the unrelated Owner in @/lib/integrations/core/types at a glance. - Keep the authoritative 'settle only after commit' explanation on settleCancelledReviews and have the three other sites that restated it near-verbatim reference it instead, so the invariant only needs updating in one place if it ever changes. * fix(github): picker action races, error-state polish, and effect churn - Disable every action in the connection-attempt picker (candidate select and install-new) while any one of them is pending, not just its own action. Before this, confirming a candidate left install-new clickable while selectConnection was still in flight (the confirm modal already closed, before the redirect actually navigated away), letting the two flows race. - Derive isNotFound as isError && <confirmed NOT_FOUND> at the call site so the two props can never disagree, instead of trusting the caller to keep them in sync independently. - Give the generic-error retry action a pending state (isRetrying, wired from the query's isFetching) and rename it from the generic 'Try again' to 'Reload connection attempt', consistent with this component's other verb+object labels. - Give both of this PR's newly added buttons (install-new, the retry action) the same touch-target class the primary button already uses, and make both of them variant=default when they are the sole action in their state, per the one-primary-action-per-surface convention this component already follows for install-new-as-sole-candidate. - OrganizationSetupWizard's focus/visibilitychange effect now reads checklistQuery.refetch through a ref updated every render and depends on [] instead of the whole query result object, avoiding needless listener churn on every render. Extends GitHubConnectionAttemptState's render tests for the renamed/ pending retry state and the cross-action disable behavior. * fix(github): reject a non-JSON OAuth token response with a clear error If GitHub (or an intermediate proxy) ever returned a 2xx response with a non-JSON body, await response.json() threw a raw, unattributed SyntaxError instead of a message identifying what actually failed. Still caught by the callback route's outer try/catch, so not a crash, but a confusing entry in logs/Sentry. Wrap the parse and surface a clear, attributed error; the normal JSON success path and the existing error/error_description surfacing are unchanged. Add the missing test case alongside the existing include/omit/error-body coverage. * docs(github): correct inaccurate rationale in disconnect review-cancellation comment The comment claimed cancelling reviews on disconnect was needed because evaluateGitHubSharingCompatibility would otherwise treat the disconnected association as still-active incumbent work. That's no longer true (and wasn't when this comment was written): sharing-compatibility.ts already excludes disconnected associations from its own association query entirely, independent of review status. The actual reason to cancel here is to avoid an orphaned queued/running review and a dangling dispatch reservation, matching the accurate rationale already given for the uninstall path. No behavior change. * refactor(github): make the picker's error state a discriminated union isError/isNotFound were two independent booleans where only 3 of 4 combinations were valid, enforced solely by a doc comment asking callers to derive isNotFound as isError && <confirmed NOT_FOUND>. Replace both with a single error: null | { kind: 'not_found' } | { kind: 'other' } prop, making the invalid 'not found but not an error' combination unrepresentable instead of relying on caller discipline. Behavior is unchanged; the one caller (OrganizationGitHubInstallations.tsx) now builds the discriminated value directly from the same tRPC error-code check it used before. Also give the candidate-select buttons the same min-h-11 sm:min-h-0 touch-target class already applied to their sibling install-new/retry buttons in this same picker, so tap targets are consistent within one button group instead of the candidates falling back to the smaller default. Add the missing test for the install-new button's own loading label ('Opening GitHub…') while isInstalling, alongside the existing tests for disabling every other action while it's in flight. * refactor(github): move latest-ref update into an effect, not the render body checklistRefetchRef.current was reassigned directly in the render body, which is a commonly-used but technically-out-of-spec pattern (React documents refs as write-during-effect/event-handler, not write-during-render). Move the assignment into its own deps-less effect that runs after every render; the listener effect below still depends on [] and still reads the ref only from its event listeners, so the focus/ visibilitychange refetch behavior is unchanged. * fix(db): reconcile orphans before validating github FK Split 0243 into a pre-flight reconciliation migration and the FK-adding migration: - 0243_reconcile_orphaned_github_installation_id.sql nulls out any platform_integrations.github_installation_id that no longer references an existing github_app_installations row, so the FK added below cannot fail its full-table validation on drift. - 0244_free_thena.sql (renamed from 0243_exotic_triton.sql) now declares the FK with an explicit ON DELETE restrict, matching the repo convention for blocking FKs and the lifecycle already enforced in code: github_app_installations rows are soft-deleted (lifecycle_state/deleted_at), and the only hard-delete path (anonymizeCloudUserData) already guards with a NOT EXISTS check against remaining associations. schema.ts documents the ON DELETE restrict choice inline. * fix(github): only record webhook receipt on 2xx dispatch dispatchSharedOnce recorded a completed delivery receipt whenever the shared-installation receipt lookup wasn't missing_canonical, regardless of the dispatched handler's response status. A non-2xx dispatch was therefore marked completed, so a later GitHub redelivery of the same event was short-circuited as a duplicate and could never retry. Only record the receipt when response.ok, leaving non-2xx dispatches unrecorded so GitHub's redelivery can retry them. missing_canonical behavior is unchanged. * fix(integrations): supersede unexpired pending OAuth attempts beginProviderOAuthAttempt only expired pending/consumed rows whose expires_at was already in the past before inserting the new attempt. An unexpired pending row for the same owner+provider (e.g. an abandoned flow) still collided with the partial unique index on (owner, provider, status = 'pending'), blocking a fresh /connect attempt for up to ATTEMPT_TTL_MS (10 minutes) and surfacing oauth_init_failed. Drop the expires_at filter for pending rows so a new attempt always supersedes an existing pending one for that owner+provider, within the same tx and owner-row lock already held. Consumed rows are still only expired once actually past their TTL, since they don't hold that unique slot. consumeProviderOAuthAttempt's pre-step doesn't insert and matches the row to consume by exact state_hash, so it doesn't share this collision pattern and is unchanged. * fix(cloud-agent): dedupe exact-duplicate repository entries results.flatMap(r => r.repositories) in fetchRepositoriesForIntegrations had no dedup after dedupeRepositories was removed, so a corrupted or duplicated cached repositories array on a single installation would surface the same repo twice for that installation. Deliberately keep one entry per (platformIntegrationId, id) rather than deduping by repo id alone: NewSessionPanel already keys and selects repository rows by platform + platformIntegrationId + id, and the existing 'preserves both association choices when two installations expose one repository' test documents that two different installations granting the same repo must both remain selectable, since session start resolves the token/permissions from the chosen installation. Deduping by repo id would silently drop one of those legitimate, functionally distinct entries. Added a test covering the same-installation duplicate case and confirmed the existing cross-installation test is unaffected. * fix(github): stop locking an incumbent personal owner out of their own installation The personal (non-org) cross-owner rejection in connectVerifiedGitHubInstallation fired whenever *any* other owner had an association for the installation, regardless of that owner's connection state and regardless of whether the caller already had their own association. So a personal owner who was the original tenant of their own installation got rejected with claimed_by_other_owner the moment an allowlisted org attached as a second tenant (running sharing admission and setting sharing_mode=web_cloud_agent), locking the incumbent out of their own connection. The 'cross-owner protection unchanged' claim held for new claimants but not for incumbent reconnects. Gate the rejection on the caller having no association of their own (!existing) AND at least one *actively connected* other-owner association. A caller reconnecting/refreshing their own existing row is not a new claimant, and a locally disconnected other-owner row is not an active incumbent (matching the disconnected-relinquish semantics the org path already uses). This does not reintroduce the sharing-admission issue the active/disconnected split originally fixed: requiresSharingAdmission is untouched and still keys off active other-owner associations only, so an org attaching as a genuine new second tenant still goes through sharing admission unchanged. Tests (github-installations.test.ts): - regression: personal incumbent -> allowlisted org attaches (sharing_mode=web_cloud_agent) -> incumbent reconnects -> ok, same association. - protection intact: a new personal claimant is still rejected claimed_by_other_owner while another personal owner, or an organization, is actively connected. - new personal claimant with only a disconnected other-owner row is no longer force-rejected and attaches as an ordinary first tenant. - an org attaching as a new second tenant still fails the unchanged sharing-admission gate. Both new behavior tests were confirmed to fail against the pre-fix code with reason claimed_by_other_owner. * fix(slack): accept adapter-written repository cache * fix(github): stop an unbound legacy association from uninstalling a shared installation upstream uninstallExclusiveGitHubInstallation resolved both the canonical sharing mode and the set of sibling tenants through the *target row's own* github_installation_id binding. For an unbound target (github_installation_id IS NULL) that made: - the shared-installation guard a no-op (canonicalId was null), and - the sibling query fall into its isNull(github_installation_id) branch, which cannot see a sibling that IS bound to the canonical. Legacy rows can be unbound: the canonical backfill in migration 0239 deliberately skipped associations that were already disconnected or suspended. So a tenant holding such a row could call uninstall and have deleteUpstream delete the GitHub App installation while another tenant was actively using it — observeGitHubInstallationLifecycle then suspends every non-disconnected association for that installation, taking the other tenant's live integration (webhooks, runtime, code reviews, bots) down with it. The pre-PR check (connected set must equal exactly the target) refused this case, so this was a regression from the earlier relinquish-aware uninstall change. Resolve the canonical by stable installation identity (github_app_type, installation_id) and match sibling tenants by the same identity (platform + platform_installation_id + effective app type, non-disconnected, excluding the target), regardless of how either row is bound. The target row is still locked owner-scoped first. Tests: an unbound legacy association can no longer uninstall while a bound sibling tenant is actively connected (upstream delete not called, sibling stays active, no suspension cascade); the shared-canonical guard now also applies to unbound targets; and an unbound legacy association with no connected sibling still uninstalls as before. Reverting the fix locally makes the two cross-tenant tests resolve instead of reject. * fix(github): harden shared delivery and repository validation * fix(cloud-agent): repair personal GitHub session start and stale refresh Three must-fix issues found in code review of the GitHub connections work: 1. NewSessionPanel's GitHub-integration guard was unconditional on selectedGitHubIntegrationId, but that field is only ever populated for organization-scoped repos. Every personal (non-org) user selecting a GitHub repo hit 'Select the GitHub repository again' and could never start a session. Scope the guard to organizationId, matching the sibling conditional already used for creationIntent. 2. Both listGitHubRepositories tRPC output schemas silently stripped githubAppType (and the personal router also stripped platformIntegrationId/platformAccountLogin), making the new 'Lite' badge dead code and contributing to (1). Added the missing fields to both schemas. 3. refreshInstallation stopped syncing account/permissions/scopes/ repository_access/installed_at onto the platform_integrations association row after the shared-installation refactor removed the legacy upsertPlatformIntegrationForOwner call. getInstallation/ listIntegrations read those columns directly off platform_integrations, so GitHubIntegrationDetails kept showing stale data after a refresh. Added syncIntegrationInstallationDetails, scoped by integration id only (safe for shared/multi-owner installations, unlike the removed legacy writer), and wired it into refreshInstallation. Added regression tests for all three: both listGitHubRepositories routers now assert a full repo object round-trips unchanged through the real tRPC output validation, and refreshInstallation's tests assert syncIntegrationInstallationDetails is called with fresh data (and not called on the early-exit failure path). * fix(github): reclaim stale installation delivery claims * fix(github): enforce webhook delivery-claim window * fix(db): make github FK migration crash-safe and guard orphan reconciliation Split 0244 into three independently-tracked migrations so a failed or interrupted deploy can never strand durably-committed DDL behind an untracked migration, and never leaves a window with neither the old nor the new uniqueness protection in place: - 0244_salty_cyclops.sql: plain transactional additions only (new tables, new columns, all FK constraints including the validating platform_integrations FK). No explicit COMMIT/BEGIN, so Postgres rolls the whole file back atomically on any failure; a retry is always clean. - 0245_calm_lockheed.sql: only the four CREATE UNIQUE INDEX CONCURRENTLY replacement indexes, fenced with the repo's approved COMMIT/BEGIN pattern. Does NOT drop the old two unique indexes. - 0246_clean_the_renegades.sql: drops the two old unique indexes and adds the sharing_mode CHECK constraint, only after 0245 has been independently tracked as applied, so the replacements are always confirmed built before the originals are removed. Verified empirically: applying only through 0245 leaves both the old and new indexes coexisting (all valid) - there is no point in the sequence where the uniqueness invariant is unprotected. 0246 then applies cleanly as its own later migration. Per packages/db/AGENTS.md, deliberately did not add IF NOT EXISTS or a pre-emptive DROP INDEX IF EXISTS around the CONCURRENTLY builds: the skill explicitly prohibits automatically masking a failed/invalid concurrent index build that way and requires explicit operator inspection and recovery (DROP INDEX CONCURRENTLY / REINDEX INDEX CONCURRENTLY) instead. The file split already achieves retry-safety at the migration-tracking level without needing that. Also hardens 0243: replace the silent UPDATE that nulled any dangling platform_integrations.github_installation_id with a DO 12662 guard that RAISEs if any orphan exists. Nulling was a behavior change in disguise - for a row with github_app_type IS NULL, clearing the reference makes it newly eligible for the legacy bot-link fallback lookup (which treats github_app_type = 'standard' OR IS NULL as equivalent). Since production orphan_count is confirmed 0, assert the invariant instead of silently mutating data/eligibility. Verified: drizzle-kit check passes; the full chain (including a staged partial-apply through 0245 only, then 0246 separately) applies cleanly from scratch against a fresh local Postgres; schema.ts is unchanged (same end-state schema, just staged across more granular migrations); packages/db/src/schema.test.ts (103 tests) passes. * fix(db): make concurrent index builds in 0245 retry-safe Each of the four CREATE UNIQUE INDEX CONCURRENTLY statements in 0245_calm_lockheed.sql had no idempotency guard. CONCURRENTLY can leave an INVALID index under its name if a build fails partway or is interrupted; a blind retry of CREATE UNIQUE INDEX CONCURRENTLY on the same name then fails with 'already exists' instead of rebuilding it - the exact non-retryable strand Finding #1 addressed, just moved down to the index-build step. Precede each build with DROP INDEX CONCURRENTLY IF EXISTS on its exact name: a no-op on the happy path (index doesn't exist yet), and a clean removal of a leftover invalid index otherwise, so every retry converges to the same end state. Per packages/db/AGENTS.md, deliberately not using CREATE ... CONCURRENTLY IF NOT EXISTS, since that would leave a leftover invalid index in place unrebuilt instead of recovering it; DROP INDEX CONCURRENTLY is the exact recovery mechanism the skill names, just applied proactively rather than only after manual diagnosis. Both DROP CONCURRENTLY and CREATE CONCURRENTLY remain in the existing post-COMMIT/pre-BEGIN section, outside any transaction block, as required. No change to the final index set, names, or schema.ts/snapshots. Verified: drizzle-kit check passes. Full chain applies cleanly from scratch. Directly simulated the failure mode: seeded a duplicate-data conflict, forced one of the four builds to fail and leave an INVALID index (confirmed indisvalid=false), confirmed a blind retry without this guard fails with 'already exists' (reproducing the bug), then repaired the data and confirmed the guarded migration now retries cleanly to a fully valid final index set. packages/db/src/schema.test.ts (103 tests) passes. * fix(github): serialize pending-install completion with sharing admission The installation.created handler read the existing-association exclusivity state, auto-completed the pending row, and bound it to the canonical installation as three separate un-serialized operations. A competing verified connection could commit between the check and the bind, leaving two active tenant associations on one canonical installation while sharing_mode stayed exclusive — the pending request became a second tenant without ever passing sharing admission, compatibility, or a fresh authorization check. Hold the installation advisory lock (the same identity lock connectVerifiedGitHubInstallation and bindGitHubIntegrationToCanonicalInstallation take) across the whole decision: re-check exclusivity, complete the pending row, and bind, all inside one transaction. If another tenant already owns the installation, the request is left PENDING and the existing 'requires verified connection confirmation' response is returned, so it must go through the verified connect-existing path. Supporting changes: exported lockGitHubInstallationIdentity so callers can join the same lock, and gave autoCompleteInstallation and bindGitHubIntegrationToCanonicalInstallation optional transaction parameters (existing callers keep their current behavior). Tests: installation-handler.test.ts updated for the new call shape; new DB-backed installation-handler.db.test.ts covers (a) a pending request staying pending when another tenant already owns the installation, (b) a winning pending completion still forcing the loser through sharing admission, and (c) a controlled-concurrency case that pauses the pending completion inside its transaction and asserts a competing verified connection cannot commit while it is in flight (verified to fail against the pre-fix handler). * fix(github): apply the pending-completion transaction to the right query The previous commit's transaction plumbing landed on the non-GitHub tail of upsertPlatformIntegrationForOwner instead of autoCompleteInstallation, which both failed typecheck (undeclared 'transaction' in that scope) and left the pending-install completion running outside the handler's transaction — so a later bind failure would still have committed the row. Point the update in autoCompleteInstallation at the supplied transaction and restore the untouched writer, and add a rollback test: when the canonical binding throws, the pending request must remain pending with no installation id applied. That test fails against the non-transactional variant. * fix(github): never route a webhook through a disconnected tenant association After two tenants share an installation, locally disconnecting one keeps its row and flips the canonical back to exclusive (only the other tenant remains connected). findIntegrationByInstallationId selects an arbitrary row for the installation with no health filter, so it can return the disconnected tenant's row. The deferred-dispatch check then authorized on installation id and app type alone, which passed on the healthy sibling — and the caller went on to log and dispatch the payload under the revoked tenant, retaining private webhook data and routing PR/cache/workflow updates to it. Validate the selected association before it is used as the delivery's owner: a retained row that is disconnected, suspended, auth-invalid, or not active is never routed (200, no logging, no dispatch) at all five routing sites. Also pass that exact association id into assertGitHubInstallationRuntimeAuthorized via isAvailableForDeferredGitHubDispatch, so installation-wide authorization can never authorize deferred work that carries a different association's identity. Tests: webhook-handler fixtures now carry a realistic health state; added the review's regression (a disconnected selected association plus a healthy sibling that authorizes must produce no logging and no dispatch — fails without the routing guard) and a check that deferred dispatch validates the exact routed association id. * fix(github): let an exact shared association read its repository inventory A newly attached shared all-repositories association starts with repositories=null, and discovery/Slack read the cached list. Two things blocked populating it: completeGitHubConnectionAttempt only fetched repositories for selected-repository installations, and both the runtime SQL and the in-memory predicate required sharing_mode=exclusive even when the caller supplied one exact association id — so a legitimately shared association could never refresh its own inventory (web discovery failed, Slack saw no repositories). Cache the inventory for both selections at attach, and allow a shared canonical only on the exact-id path. The carve-out keeps every other check: ownership (org membership / user ownership), local health (active, not suspended, not auth-invalid, not disconnected), and canonical health (active, not suspended, not deleted, not auth-invalid). The generic, installation-wide path (no exact id) still requires exclusive, so the runtime-token guard is unchanged for callers that did not resolve a tenant. Tests: added the review's shared-repository regression (real adapter and runtime predicate with a healthy shared association and exact id), exact-id shared authorization vs the still-rejecting generic path, enforcement of local and canonical health and owner validity on the exact-id path, and an attach-path test proving an all-repositories attach caches the fetched inventory. * chore(github): apply formatter output to the security-fix files * fix(github): treat an empty exact-association id as the exclusive-only generic path assertGitHubInstallationRuntimeAuthorized enabled the shared-installation carve-out from the PRESENCE of expectedIntegrationId (expectedIntegrationId !== undefined) while the exact-id query predicate and the row limit used its TRUTHINESS. An empty-string id therefore enabled shared mode without constraining the query to an exact association, so a shared canonical installation could be authorized installation-wide through one healthy association — breaking the invariant that shared mode is permitted only with a genuine exact id. Normalize once into exactAssociationId and drive the predicate, the limit, and the carve-out from that single value, so an empty string is the generic exclusive-only path. No other behavior changes: the non-empty exact-id carve-out and the ownership/local/canonical health checks are untouched. Test: 'treats an empty exact-association id as the generic exclusive-only path' — a shared installation with one healthy association and a suspended sibling rejects an empty-string id (verified it resolved before this fix) while the real exact id still resolves. * fix(github): restore exact-id shared runtime authorization lost in the main merge The main #6178/#6179 reconciliation merge (81628eea1) kept sharing_mode gating unconditionally exclusive-only, which regressed a shared (web_cloud_agent) association's ability to ever authorize, even with an exact association id pinned. Note: this carve-out never actually existed on feat/github-connections-pr2 before that merge (verified via git history: it only exists on feat/github-connections-pr4's commits 15ae07032 and f53e068d2) — but it is a real, needed capability for shared installations to read/refresh their own repository inventory, so re-applying it here now, adapted to main's reason-returning shape: 1. getGitHubRuntimeAssociationRejectionReason(association, options?) takes { allowShared?: boolean }; sharing_mode is only permitted to be 'web_cloud_agent' when allowShared is explicitly true, otherwise it must be 'exclusive'. isGitHubRuntimeAssociationAuthorized forwards the same options (defaulting to exclusive-only). 2. assertGitHubInstallationRuntimeAuthorized normalizes expectedIntegrationId once into exactAssociationId (empty string -> undefined) and derives allowShared = exactAssociationId !== undefined, so the id predicate, the row limit, and the sharing-mode carve-out can never diverge (the F3 invariant). 3. The SQL's canonical-installation OR-branch permits sharing_mode IN (exclusive, web_cloud_agent) only when allowShared; otherwise exclusive only. 4. The post-query reason check passes { allowShared } through so it agrees with what the SQL already filtered for. Every other check (ownership, local health, canonical installation health, ambiguous_association for the generic path) is unchanged. Ported the PR4 test coverage for this onto PR2's reason-based API: - runtime-authorization.test.ts: allowShared true/false/unset on the pure reason function and boolean wrapper, plus mock-level exact-id vs generic-path authorization of a shared association. - github-installations.test.ts: a real end-to-end integration test (two orgs sharing one installation) covering exact-id shared authorization, generic-path rejection, an added empty-string-id F3 case, unknown-id rejection, and local/owner health enforcement on the exact-id path; plus canonical-installation-unhealthy rejection. - New review-shared-repositories.test.ts (ported from PR4): an end-to-end adapter-level regression proving fetchGitHubRepositories succeeds for an exact healthy shared association. Did not port PR4's separate connection-service.ts repository-caching- at-attach fix (15ae07032) — that is an unrelated bug in a different function, out of scope for this regression fix. Verified: pnpm --filter web typecheck clean; oxlint clean on all 4 changed files; pnpm format (line-wrap only); git diff --check clean; no schema.ts or migration changes. 182 tests pass across the full required suite (runtime-authorization, review-shared-repositories, webhook-handler + webhook-handler-dispatch.integration, installation-handler, connection-service, github-integration-helpers, prepare-review-payload, github-installations). * fix(github): let installation.unsuspend clear a suspended association The F2 routability guard was applied to every routing branch, including installation.unsuspend. isRoutableGitHubIntegration rejects unhealthy rows and a suspended row is unhealthy, so the guard dropped the one event that can clear the suspension: with connection management disabled (the default) the suspend webhook marks the association suspended, and the later unsuspend delivery was then refused with 'Integration unavailable', leaving the association and its canonical installation suspended until a manual reconnect. Unsuspend is a recovery event, so gate that branch on disconnected-ness only (isRecoverableGitHubIntegration): a suspended row stays routable while a locally disconnected former tenant is still refused, preserving the F2 protection. The other branches keep the full health guard. handleInstallationUnsuspend also resolved its owner through an unfiltered lookup, so a disconnected former tenant's row could still be the association an unsuspend was applied to; it now ignores a locally disconnected row while keeping a suspended row eligible. Tests: webhook-handler 'processes installation.unsuspend for a suspended association instead of dropping it' (fails with 'Integration unavailable' under the old guard) and 'still refuses to route installation.unsuspend for a locally disconnected association'; DB-backed installation-handler tests 'recovers a suspended association when GitHub delivers installation.unsuspend' (asserts it becomes active with suspended_at cleared) and 'does not unsuspend a locally disconnected former tenant'. * fix(github): keep the all-repositories inventory sync best-effort The F3 change awaited fetchGitHubRepositoriesForMaintenance unconditionally before the connection transaction, so a transient listing failure (rate limit, 5xx, timeout) aborted completeGitHubConnectionAttempt and the connection was never created — previously an all-repositories attach never depended on that call at all. Restore best-effort semantics for the all-repositories case: the fetch still runs (so the cache is populated when it succeeds, which is F3's intent), but a failure is captured and the attach proceeds with repositories left null for a later refresh. The selected-repositories case keeps surfacing the failure, because that listing is the user's own selection. This matches how the pending-install completion path already treats the same call. Test: 'a failing inventory listing does not block an all-repositories attach' — the attach completes with an active association and null repositories (aborts before this fix); the existing caching test still asserts the inventory is populated on success. * fix(github): serialize the legacy install bind against pending completion A pending-install completion could still attach a second tenant to an installation the legacy (connection-management-disabled) callback was already claiming. That path commits the association unbound and binds it in a separate step, and the webhook's locked existence check only counted CANONICALLY BOUND rows — so a peer that was committed but not yet bound was invisible, and a second organization could be auto-completed and bound onto the same canonical installation with sharing_mode left exclusive. - Broaden the locked check in handleInstallationCreated to count every other live association for this installation (pending or active, bound or not, excluding the pending row and locally disconnected rows), so an unbound legacy peer blocks the completion instead of being skipped. - Serialize the legacy leg: the install callback's create+bind now runs under the same installation advisory lock with the same competing-tenant check, and refuses with 'installation_already_claimed' rather than double-binding. - Share one app-type predicate (effectiveAppTypeCondition, now exported) instead of a third copy. Tests: installation-handler.db 'does not auto-attach a second tenant while legacy associations are committed but unbound' (the pending row became active before this fix) and the callback route suite updated for the locked bind. * fix(github): apply lifecycle events to the connected tenant, not an arbitrary row Canonical lifecycle handlers resolved their tenant with findIntegrationByInstallationId, which returns an arbitrary association for the installation, and unsuspend then discarded the result when it happened to be locally disconnected. After one tenant disconnects and another stays connected, the lookup can return the retained disconnected row, so the event was dropped and the connected tenant's suspension never cleared — the association and its canonical installation stayed suspended until a manual reconnect. Suspend had the same arbitrary-row selection. Resolve the tenant through a new findLifecycleIntegration: keep using the existing lookup first (so behaviour is unchanged when it already returns a connected row), and when it returns a locally disconnected former tenant fall back to an explicit connected-association query for the installation and app type. suspend and unsuspend both use it; canonical state (observeGitHubInstallationLifecycle) still runs unconditionally. A locally disconnected tenant is never the association acted on, so the earlier never-act-on-a-disconnected-tenant fix is not reopened. Test: installation-handler.db 'recovers the connected tenant when a disconnected former tenant is also present' — with a disconnected row inserted first, the connected suspended tenant stayed suspended before this fix and now becomes active. * chore(github): apply formatter output to the lifecycle regression test * fix(github): resolve the connected association for installation.unsuspend findIntegrationByInstallationId has no health filter and no ordering, so for an installation with a retained locally disconnected row it can return that row instead of the tenant that still owns the installation. The route guard then dropped the unsuspend delivery before the handler ran, and the handler discarded the disconnected row, so the live tenant's suspension was never cleared — the same stuck state, still reachable. Add findConnectedIntegrationByInstallationId (shared condition builder with findIntegrationByInstallationId, plus github_disconnected_at IS NULL) and use it for the unsuspend path in both places: the webhook route resolves the connected tenant for logging and dispatch, and the handler resolves the connected tenant for the tenant-facing unsuspend, instead of resolving an arbitrary row and discarding it. Suspend uses the same connected resolution. Canonical state (observeGitHubInstallationLifecycle) still runs unconditionally, and a locally disconnected tenant is never acted on. Tests: webhook-handler 'does not log installation.unsuspend when only a disconnected former tenant remains' replaces the drop-the-event assertion, and the suspended/connected cases assert the connected lookup is used; installation-handler mocks updated to the new lookup. * fix(github): gate the legacy bind guard and compare owners NULL-safely Two defects in the competing-tenant guard added for the legacy install path: - It ran unconditionally, so a connection-management-ENABLED install was rejected with installation_already_claimed even though connectVerifiedGitHubInstallation had already admitted and committed the shared association. The guard now only runs on the legacy (management-disabled) writer path, where the unbound-commit-then-bind window exists; the verified path binds directly as before. - The owner comparison used ne() against nullable columns, and is NULL, so a competitor owned by the other owner type (for example an org-owned row when the caller is user-owned) was invisible and the guard would bind anyway, reopening the two-tenants race. NULL is now treated as a different owner explicitly. Tests (callback route suite, DB-backed): 'does not block a management-enabled install when a shared tenant already owns the installation' (returned installation_already_claimed before the gating fix) and 'refuses the legacy bind when a competitor of the other owner type owns the installation' (bound before the owner-comparison fix). * fix(github): make the legacy install write, guard, and bind one transaction The legacy (management-disabled) callback committed the association through upsertPlatformIntegrationForOwner in its own transaction and only then ran the competing-tenant guard and the canonical bind in a second transaction. The advisory lock serialized admission, but a bind failure or an interrupted request left a committed active-but-unbound association behind even though an error was returned, and a concurrent pending-install completion could then attach a second tenant. The legacy path now runs lockGitHubInstallationIdentity, the NULL-safe cross-owner competitor check, the write, observeGitHubInstallationLifecycle, the owner-scoped read, and the bind inside ONE db.transaction, so any failure rolls the whole thing back and the caller still gets today's installation_already_claimed / error redirect. upsertPlatformIntegrationForOwner and findIntegrationByInstallationIdForOwner gained optional transaction parameters (the same pattern autoCompleteInstallation and the bind already use) so the writer joins the caller's transaction instead of opening its own. The verified-writer path is untouched: it still uses its own writer and binds directly. Tests (callback route suite, DB-backed): 'rolls back the legacy write when the canonical bind fails' (the real writer runs inside the caller transaction; the failed bind leaves no association row) and the competitor test now also asserts nothing was written for the caller. * fix(github): stop the suspend route from dropping a lifecycle delivery installation.suspend in the webhook route still resolved an arbitrary association with findIntegrationByInstallationId and early-returned Integration unavailable through isRoutableGitHubIntegration when that row happened to be a retained disconnected former tenant. handleInstallationSuspend therefore never ran, so the canonical installation state was never reconciled — the same class already fixed for unsuspend, but only in the handler. Mirror the unsuspend fix: the suspend route resolves the connected association with findConnectedIntegrationByInstallationId and no longer applies the generic routability early-return, so the handler always runs and reconciles canonical state even when only a disconnected tenant remains. The handler still restricts the tenant-facing suspend to the connected, non-disconnected association, so never-act-on-a-disconnected-tenant is preserved. Tests: 'dispatches installation.suspend for the connected tenant when a disconnected former tenant exists' (asserts the unfiltered lookup is not used, the delivery is logged, and the handler runs) and 'still dispatches installation.suspend when only a disconnected former tenant remains' (handler runs for canonical state, nothing logged against the disconnected tenant). Both fail against the pre-fix route.

… (part 1/1) (#6282) ## Changelog for users - Google Play purchases now record the amount actually charged, the currency, and the tax withheld in `kilo_pass_store_purchases`. - Warehouse revenue can be calculated per Google Play purchase from the recorded amount and currency. - New purchases take the three values from the Google Play order data the platform already receives. - A one-off run fills the three values for existing Google Play purchases from their stored order ids. - A purchase whose Play order carries no money, or money that cannot be read, still completes and records nothing rather than a wrong amount. ## Changelog for maintainers - `kilo_pass_store_purchases` gains nullable `amount_charged_minor_units` (integer), `currency` (text), and `tax_minor_units` (integer); migration 0250 is additive and adds check constraints for non-negative amounts, `^[A-Z]{3}$` currency, and a currency whenever an amount is present. - The Play order request now asks for order- and line-item-level `total` and `tax`; the same call feeds verification, notifications, and the backfill. - New `store-purchase-money.ts` converts Play `Money { currencyCode, units, nanos }` to the currency's smallest unit using its ISO exponent; it prefers the matched line item, then the only line item, then order totals, and maps anything unusable to nulls. - The Google Play verifier attaches the money to the mapped purchase, and the single insert in `completeStoreKiloPassPurchase` persists it; App Store rows stay NULL. - New `backfillGooglePlayPurchaseAmounts` re-reads each stored order id, updates only rows with both money columns NULL, and is idempotent and resumable via `limit`; failures carry row id, order id, and message. - The backfill script supports `--limit`, `--dry-run`, and `--help`, and validates the Play service account before the first request so a bad credential exits non-zero instead of reporting only a failure count. - Review first: the currency-exponent fallback for well-formed but unknown codes (treated as two decimals), and the rule that an item carrying unreadable money yields nulls instead of borrowing order totals. ## E2E proof **[e10] ux-check: Kilo Pass purchase/checkout and account surfaces regression** — e10-surfaces.log records '/subscriptions' rendering 'Kilo Pass / KiloClaw / Coding Plans' and '/profile' rendering the Kilo Pass plans ('Starter', 'Pro', 'Expert', 'Buy now'), and e10-checkout.log records 'after buy url=https://checkout.stripe.com/c/pay/cs_test_' with 'Subscribe to Kilo Pass (Starter)' (screenshots e10-subscriptions.png, e10-profile.png, e10-checkout.png); the only error state seen, a 500 on /api/auto-routing/settings (e10-autorouting.log), is tagged [pre-existing] — no auto-routing file appears in e10-diffstat.log — and presents a 'Try again' action. [e10] ux-check: Kilo Pass purchase/checkout and account surfaces regression — e2e-web/e10-profile.png (capture not retained) **[e10] ux-check: Kilo Pass purchase/checkout and account surfaces regression** [e10] ux-check: Kilo Pass purchase/checkout and account surfaces regression — e2e-web/e10-checkout.png (capture not retained) **[e10] ux-check: Kilo Pass purchase/checkout and account surfaces regression** ![[e10] ux-check: Kilo Pass purchase/checkout and account surfaces regression — prior/e10-profile.png](https://github.com/user-attachments/assets/3719d1d1-1a52-431b-8c30-3718765788ae) **[e10] ux-check: Kilo Pass purchase/checkout and account surfaces regression** ![[e10] ux-check: Kilo Pass purchase/checkout and account surfaces regression — prior/e10-checkout.png](https://github.com/user-attachments/assets/2661bfe5-6c5b-4dd6-868b-ad8c4b343700) ## E2E proof — log excerpts ``` [e2] ux-check: Kilo Pass purchase/checkout and account surfaces regression -> pass :: Live playwright run against the dev stack at localhost:4800 signed in as e2e-verifier@kilo.test: /subscriptions renders 'Manage your subscriptions and billing in one place.' with the Kilo Pass, KiloClaw ('New KiloClaw instances are unavailable') and Coding Plans ('MiniMax Token Plan Plus' ... 'Sold out') tabs, /profile renders 'Unlock up to 50% free credits. Checkout securely with Stripe.' and the purchase entry is present ('buynow count=3' in e2-surfaces.log), the first Buy now reached the hosted checkout shown as 'Subscribe to Kilo Pass (Starter)' (e2-surfaces.log), no visible element is new or changed because the diff touches no UI file (e2-diffstat.log), and no layout shift was measured: ``` <details><summary><code>/home/igor_kilocode_ai/.local/share/kwf/sections/req-20260917-192220-8435/e2e-web/e2-surfaces.log</code></summary> ``` Maldives Mali Malta Martinique Mauritania Mauritius Mayotte Mexico Moldova Monaco Mongolia Montenegro Montserrat Morocco Mozambique Myanmar (Burma) Namibia Nauru Nepal Netherlands New consoleErrors: ["Failed to load resource: the server responded with a status of 401 (Unauthorized)","Failed to load resource: the server responded with a status of 404 (Not Found)","Failed to load res 5xx/4xx responses: ["401 http://localhost:4800/api/user","404 http://localhost:4800/ingest/array/fake-token/config.js","404 http://localhost:4800/ingest/array/fake-token/config?ip=0&_=1789672743291&ve DONE ``` </details> <details><summary><code>/home/igor_kilocode_ai/.local/share/kwf/sections/req-20260917-192220-8435/e2e-web/e2-diffstat.log</code></summary> ``` apps/web/src/lib/kilo-pass/store-subscription-completion.ts apps/web/src/scripts/d2026-09-17_backfill-store-purchase-amounts.ts packages/db/src/migrations/0250_kilo_pass_store_purchase_amounts.sql packages/db/src/migrations/meta/0250_snapshot.json packages/db/src/migrations/meta/_journal.json packages/db/src/schema.ts $ git diff --stat 4238fe9..HEAD .../kilo-pass/google-play-notifications.test.ts | 70 + apps/web/src/lib/kilo-pass/google-play-sdk.test.ts | 29 +- apps/web/src/lib/kilo-pass/google-play-sdk.ts | 36 +- .../src/lib/kilo-pass/google-play-verifier.test.ts | 39 + apps/web/src/lib/kilo-pass/google-play-verifier.ts | 2 + .../store-purchase-money-backfill.test.ts | 355 + .../lib/kilo-pass/store-purchase-money-backfill.ts | 184 + .../src/lib/kilo-pass/store-purchase-money.test.ts | 185 + apps/web/src/lib/kilo-pass/store-purchase-money.ts | 108 + .../store-subscription-completion.test.ts | 138 + .../lib/kilo-pass/store-subscription-completion.ts | 6 + .../d2026-09-17_backfill-store-purchase-amounts.ts | 123 + .../0250_kilo_pass_store_purchase_amounts.sql | 7 + packages/db/src/migrations/meta/0250_snapshot.json | 41877 +++++++++++++++++++ packages/db/src/migrations/meta/_journal.json | 7 + packages/db/src/schema.ts | 19 + 16 files changed, 43180 insertions(+), 5 deletions(-) ``` </details> <details><summary><code>/home/igor_kilocode_ai/.local/share/kwf/sections/req-20260917-192220-8435/e2e-web/e2-layoutshift.log</code></summary> ``` /subscriptions cls=0.0009420437282986111 shiftEntries=2 firstShifts=[0.0001,0.0008] /profile cls=0.00007263183593750001 shiftEntries=1 firstShifts=[0.0001] ``` </details> <details><summary><code>/home/igor_kilocode_ai/.local/share/kwf/sections/req-20260917-192220-8435/e2e-web/e2-autorouting.log</code></summary> ``` status=500 body(head 1200)= ``` </details> <details><summary><code>/home/igor_kilocode_ai/.local/share/kwf/sections/req-20260917-192220-8435/e2e-web/web-setup-smoke.log</code></summary> ``` ✘ 1 [chromium] › tests/setup-smoke/profile.spec.ts:10:7 › local setup smoke › signs in with fake auth and renders the profile page (30.2s) 1) [chromium] › tests/setup-smoke/profile.spec.ts:10:7 › local setup smoke › signs in with fake auth and renders the profile page Test timeout of 30000ms exceeded. Error: expect(locator).toBeVisible() failed Locator: getByRole('region', { name: 'User profile' }) Expected: visible Error: element(s) not found Call log: - Expect "toBeVisible" with timeout 30000ms - waiting for getByRole('region', { name: 'User profile' }) 58 | 59 | const profileCard = page.getByRole('region', { name: 'User profile' }); > 60 | await expect(profileCard).toBeVisible({ timeout: 30_000 }); | ^ 61 | await expect(page.getByRole('link', { name: 'Your Profile' })).toBeVisible(); 62 | await expect(profileCard.getByRole('button', { name: 'Edit profile' })).toBeVisible(); 63 | await expect(profileCard.getByText(testEmail, { exact: true })).toBeVisible(); at /home/igor_kilocode_ai/.local/share/kwf/wt/req-20260917-192220-8435/apps/web/tests/setup-smoke/profile.spec.ts:60:31 attachment #1: screenshot (image/png) ────────────────────────────────────────────────────────── test-results/setup-smoke/profile-local-setup-smoke--f6e7e-nd-renders-the-profile-page-chromium/test-failed-1.png ──────────────────────────────────────────────────────────────────────────────────────────────── Error Context: test-results/setup-smoke/profile-local-setup-smoke--f6e7e-nd-renders-the-profile-page-chromium/error-context.md 1 failed ``` </details> <details> <summary>Owner request for the stack</summary> > Add the actual amount charged, the currency, and the tax to our Google Play purchase records in kilo_pass_store_purchases. Today those records cannot tell the warehouse what a purchase really earned: we need the real amount charged, the currency it was charged in, and the tax withheld, recorded per Google Play purchase alongside what is already stored, so revenue can be calculated in the warehouse. Find the existing table and the Google Play purchase write path, extend the record with the three values, populate them for new purchases from the Google Play purchase data we already receive, and backfill what can be backfilled from the stored receipts. Prove it end to end with a real Google Play purchase record written and read back showing the amount, the currency and the tax. </details> ## PR stack (merge bottom to top) - part 1/1: #6282 (`kwf/req-20260917-192220-8435-l1`) ← this PR

Explorer finding: pr-open-by-url: The uppercase repository slug wraps to two lines and leaves the PR number '#1' orphaned on a third line, crowding the share and Submit review controls. The user-agent explorer found this while using the app like a user. One finding per item; the explorer never edits product code. Flow: pr-open-by-url Found on revision: f2181ae Repro: 1. set this state first: credits 25; reviews 4; org 2. open the app on 1F6F1503-9C26-4120-82D4-5F8768CBE42F 3. reach pr-open-by-url 4. the capture shows the defect named below Observed: The uppercase repository slug wraps to two lines and leaves the PR number '#1' orphaned on a third line, crowding the share and Submit review controls. Expected: the screen renders without this defect Evidence (from the device run): - ~/.local/share/kwf/findings/explorer-pr-open-by-url-the-uppercase-repository-slug-wra-afba1609/pr-open-by-url.png

RSO added 10 commits February 4, 2026 09:14

feat(bot): add tool system barrel export with auto-registration

282ef1f

Create index.ts that: - Re-exports all types, registry, and loader functions - Auto-registers spawn_cloud_agent tool on module import - Provides usage documentation in JSDoc

Remove unneeded import

5e33ec0

Query active integrations from database

90f7991

Bring bac ktype

efaf995

Simplify

d10f366

github-actions Bot added the stale label Mar 7, 2026

RSO closed this Mar 9, 2026

This was referenced Mar 17, 2026

Bug: Deleting a town doesn't clean up TownDO, TownContainerDO, or AgentDOs — resources leak indefinitely #1182

Closed

fix(gastown): restore agent working status on heartbeat after dispatch timeout race #1359

Merged

jrf0110 mentioned this pull request Apr 3, 2026

feat(gastown): Workflows — visual DAG builder with scheduled and event-triggered execution #1943

Closed

42 tasks

kilo-code-bot Bot mentioned this pull request Apr 8, 2026

feat(exa): add $10/month free allowance with credit billing for overages #2169

Merged

markijbema mentioned this pull request May 19, 2026

feat(gateway): route and persist model experiments #3325

Merged

kilo-code-bot Bot mentioned this pull request Jun 15, 2026

[Gastown] GitHub authentication fails for bkg.rocks repository - all PR pushes blocked #4016

Closed

specimba mentioned this pull request Jun 18, 2026

[Gastown] HERMES NEXUS patrol creates infinite triage batch beads - cannot be stopped by Mayor #3944

Closed

fanajib5 mentioned this pull request Jun 19, 2026

[Gastown] Triage dispatch infinite loop: polecats lack triage role (403), escalations self-regenerate #4132

Closed

This was referenced Jul 16, 2026

feat(mobile): paginate agent session history #4562

Merged

[Gastown] Dispatch scheduler silently fails all polecats in one rig — different from #1632, #1997, #2260 #4647

Closed

kilo-code-bot Bot mentioned this pull request Jul 28, 2026

feat(mobile): search input also filters the Active now tray #4823

Merged

2 tasks done

iscekic mentioned this pull request Jul 30, 2026

feat(mobile): PR review discussion UX + toast entry feedback #4883

Merged

2 tasks done

This was referenced Jul 31, 2026

feat(attachments): support remote CLI file delivery #4944

Merged

fix(mobile): stream and event durability #4987

Merged

This was referenced Aug 27, 2026

Refactor Slack bot to use Chat SDK for multi-platform support #553

Closed

fix(kilo-pass): prevent duplicate checkout sessions #5533

Merged

This was referenced Sep 15, 2026

fix(mobile): open GitLab and Bitbucket review links in-app #6185

Merged

fix(mobile): make the session context circle tappable in every state #6190

Merged

This was referenced Sep 21, 2026

fix(mobile): keep the PR review repository eyebrow on one line #6453

Merged

fix(mobile): keep session header trailing controls inside the viewport #6513

Merged

fix(mobile): stop the composer clipping the first draft line and cap the session header #6522

Merged

iscekic mentioned this pull request Sep 22, 2026

fix(mobile): compact session header rows and dedupe session filter options #6562

Merged

iscekic mentioned this pull request Oct 3, 2026

fix(sessions): link a pull request only on the session's verified own evidence #6968

Merged

iscekic mentioned this pull request Oct 11, 2026

mobile: decide how to handle ExpoModulesJSI promise continuations leaving the JS thread in Start agent #7403

Open

iscekic mentioned this pull request Oct 19, 2026

fix(mobile): clarify security agent permission and trusted hosts empty copy #8015

Merged

Navigation Menu

Repository navigation

refactor(bot): Extract platform-agnostic tool system - #1

refactor(bot): Extract platform-agnostic tool system#1
RSO wants to merge 10 commits into
mainfrom
refactor/bot-tool-system

RSO commented Feb 4, 2026

Uh oh!

github-actions Bot commented Mar 7, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

RSO commented Feb 4, 2026

Summary

Changes

New Tool System (/src/lib/bot/tools/)

Updated slack-bot.ts

Architecture

Testing

PR Stack

Uh oh!

github-actions Bot commented Mar 7, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

New Tool System (`/src/lib/bot/tools/`)

Updated `slack-bot.ts`