Repository navigation
feat(github): share installations with web Cloud Agents - #5978
Merged
Merged
Conversation
pandemicsyn
force-pushed
the
feat/github-connections-pr2
branch
2 times, most recently
from
September 9, 2026 16:36
fa6817f to
d70becc
Compare
pandemicsyn
marked this pull request as ready for review
September 9, 2026 17:39
Contributor
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe incremental commit Overview
Issue Details (click to expand)WARNING
Note: the inline comment for this finding could not be attached because a stale PENDING review (id 5158007287, commit d70becc) on this PR blocks new review creation (HTTP 422, Files Reviewed (5 files)
Fix these issues in Kilo Cloud Previous Review Summaries (16 snapshots, latest commit 3dcd749)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 3dcd749)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe migration-only incremental changes (0243 pre-flight orphan guard, the 0244 split, the retry-safe concurrent index rebuild in 0245, and the 0246 index drops plus Overview
Issue Details (click to expand)WARNING
Note: the inline comment for this finding could not be attached because a stale PENDING review (id 5158007287, commit d70becc) on this PR blocks new review creation. Files Reviewed (5 files)
Fix these issues in Kilo Cloud Previous review (commit 12b0e56)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe latest commit's three fixes (personal GitHub guard scoping, tRPC output schemas, and association detail resync) are correct; the one remaining finding is the fail-closed candidate guard in Overview
Issue Details (click to expand)WARNING
Note: the inline comment for this finding could not be attached because a stale PENDING review (id 5158007287, commit d70becc) on this PR blocks new review creation. Files Reviewed (8 files)
Fix these issues in Kilo Cloud Previous review (commit 242eb98)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe dedupe fix in the changed commit is correct and the two prior webhook/OAuth-attempt fixes remain verified; the previously reported fail-closed guard in the GitHub bot-link resolver ( Overview
Issue Details (click to expand)WARNING
Files Reviewed (4 files)
Fix these issues in Kilo Cloud Previous review (commit 731e775)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryIncremental review verified both fixes in this commit (webhook receipts are now recorded only on Overview
Issue Details (click to expand)WARNING
Files Reviewed (5 files)
Previous review (commit ec51eee)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryIncremental review of the migration-hardening commit found no new issues in the changed code; the previously reported Overview
Issue Details (click to expand)WARNING
Files Reviewed (7 files)
Previous review (commit af1734d)Status: 1 Issue Found | Recommendation: Address before merge Executive Summary
Overview
Issue Details (click to expand)WARNING
Files Reviewed (5 files)
Inline publication was blocked: this PR has an existing bot pending review (id 5158007287) that this session is not permitted to clear, so the finding is recorded here. Previous review (commit fda919a)Status: 1 Issue Found | Recommendation: Address before merge Executive Summary
Overview
Issue Details (click to expand)WARNING
Files Reviewed (9 files)
Inline publication was blocked: this PR has an existing bot pending review (id 5158007287) that this session is not permitted to clear, so the finding is recorded here. Previous review (commit 4c3d38b)Status: No Issues Found | Recommendation: Merge Executive SummaryRebase onto current main left GitHub-installation review scope unchanged; overlapping files differ only by already-landed main content, with high confidence. Files Reviewed (3 files)
Previous review (commit 98af34e)Status: No Issues Found | Recommendation: Merge Executive SummaryLock-observation tests now match the exact advisory-lock SQL already used by sibling suites; the single-file incremental change looks correct with high confidence. Files Reviewed (1 file)
Previous review (commit fbc73e0)Status: No Issues Found | Recommendation: Merge Executive SummaryLifecycle lock-overlap tests now wait for a blocked owner advisory lock before releasing the holder; the single-file incremental change looks correct with high confidence. Files Reviewed (1 file)
Previous review (commit f0df914)Status: No Issues Found | Recommendation: Merge Executive SummaryRepository refresh now follows the lifecycle lock order and skips deleted or inactive projections; the two-file incremental change looks correct with high confidence. Files Reviewed (2 files)
Previous review (commit 622241e)Status: No Issues Found | Recommendation: Merge Executive SummarySynchronous GitHub lifecycle cleanup now records completed receipts only after required DB work; the incremental simplification looks correct with high confidence. Files Reviewed (8 files)
Previous review (commit cd4e7d7)Status: No Issues Found | Recommendation: Merge Executive SummaryIn-lease GitHub lifecycle receipts now return retryable 503 with Retry-After; the five-file incremental change looks correct with high confidence. Files Reviewed (5 files)
Previous review (commit f1bbbb4)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryFailed GitHub Overview
Fix these issues in Kilo Cloud Issue Details (click to expand)WARNING
Files Reviewed (5 files)
Previous review (commit 458e94b)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryFailed GitHub Overview
Fix these issues in Kilo Cloud Issue Details (click to expand)WARNING
Files Reviewed (19 files)
Previous review (commit d70becc)Status: 3 Issues Found | Recommendation: Address before merge Executive SummaryExclusive GitHub uninstall retries can skip cleanup after unlink failures, unpinned runtime auth stays broken after sharing demotion, and the migration rebuilds live Overview
Fix these issues in Kilo Cloud Issue Details (click to expand)WARNING
Files Reviewed (50 files)
[Snapshot truncated.] Additional previous summary content was truncated to keep this comment within platform limits. Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0 Review guidance: REVIEW.md from base branch |
pandemicsyn
force-pushed
the
feat/github-connections-pr2
branch
from
September 10, 2026 15:26
98af34e to
4c3d38b
Compare
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.
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.
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.
Resolved apps/web/src/components/cloud-agent-next/NewSessionPanel.tsx by taking main's refactored submit-path structure (hoisted creationIntent/creationInput/bitbucketRepo memos, firstChatCreationOperation as state) while restoring PR2's shared-GitHub-installation additions: - RepositoryOption.githubAppType field - githubAppType mapping onto GitHub repo options - explicit "Select the GitHub repository again" guard before creationIntent is used in handleStartSession - githubAppType included in the repo command-item search value - the "Lite" badge for lite GitHub App installations ENVIRONMENT.md auto-merged cleanly.
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).
…liation 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.
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.
iscekic
approved these changes
Sep 14, 2026
Reconciles main's #6178/#6179 refactor of apps/web/src/lib/integrations/github/runtime-authorization.ts (reason-enum diagnostics) with this branch's canonical-installation health and sharing_mode=exclusive gating, into one coherent file: Kept from main: - GitHubRuntimeAuthorizationRejectionReason union and getGitHubRuntimeAssociationRejectionReason(association) returning a specific reason or null. - GitHubRuntimeAuthorizationError(reason, diagnostics) with { installationId, appType, integrationIds }. - isGitHubRuntimeAssociationAuthorized as a boolean wrapper (=== null). - The new caller in prepare-review-payload.ts (threaded the exact platform_integrations id it already has in scope as expectedIntegrationId, since generateGitHubInstallationToken already supported that third argument). Kept from this branch (extended the reason function to cover it, rather than duplicating it separately): - Two new reasons, installation_unavailable and sharing_not_allowed, covering canonical installation lifecycle_state/suspended_at/ deleted_at/auth_invalid_at and sharing_mode. - assertGitHubInstallationRuntimeAuthorized's third argument (expectedIntegrationId?: string), the notExists-canonical guard for unbound associations, and .limit(expectedIntegrationId ? 1 : 2). - All of the original SQL candidacy+health filtering (association integration_status/disconnected_at/suspended_at/auth_invalid_at, and the canonical join's lifecycle_state/sharing_mode/suspended_at/ deleted_at/auth_invalid_at). Initially tried moving these into the reason function only (matching main's diagnostics-first philosophy), but apps/web/src/lib/integrations/db/github-installations.test.ts's 'keeps local disconnect separate from shared upstream suspension recovery' integration test caught a real regression: without the SQL-level filters, a disconnected former owner's stale row and a still-healthy owner's row both come back and get misclassified as ambiguous_association instead of resolving to the healthy one. This is a genuine authorization-outcome change, not just a diagnostics change, so the original filters stay in SQL; the reason function's checks remain as a correctness backstop for callers (like prepare-review-payload) that fetch an association through a different, unfiltered path. Explicitly out of scope, left for PR4: expectedIntegrationId/allowShared normalization and the sharing_mode = 'web_cloud_agent' exact-id carve-out described for 'our stack' do not actually exist yet on this branch (verified: they only exist in feat/github-connections-pr4's own commits 15ae070 and f53e068). Per instructions not to touch PR3/PR4, this merge does not add them; PR4 will layer that carve-out on top of this file when it rebases. The empty-string-expectedIntegrationId-behaves- as-generic invariant already holds today, unprompted, since the existing ternaries () treat an empty string as falsy. Test reconciliation: the auto-merged test file combined both sides' association fixtures/assertions without a textual conflict, but still had one main-only case ('ignores canonical %s lifecycle and health markers') asserting the old shadow-state behavior this merge replaces; rewrote it to assert rejection. Added rejectionCases coverage for the two new reasons, and two focused tests for the exact-id vs empty-string-id call shapes. Fixed one main-only DB-mock test's db.select call-count expectation (1 -> 2) to account for the notExists subquery. Verified: pnpm --filter web typecheck clean; oxlint clean on all touched files; 174 tests pass across the full affected auth/webhook suite (runtime-authorization, webhook-handler + webhook-handler-dispatch, installation-handler, connection-service, github-integration-helpers, prepare-review-payload, github-installations), plus platform-integrations, github-installation-uninstall, bot/platforms/github, github-link-state, and both cloud-agent-next-router suites (320 tests) for related call sites. No 'review-shared-repositories' test file exists in the repo under that name; ran the closest matching suites instead.
…e 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).
This was referenced Sep 15, 2026
Resolves a migration-number collision: main added 0243_ambiguous_namor and 0244_many_vermin (organization_service_fee_exemptions, stripe_service_fee_assessments) at the same indices our stack already used. Kept main's 0243/0244 as-is and renumbered our five migrations above them via the drizzle workflow (schema.ts staged through the same sequence used to originally generate them, then pnpm drizzle generate at each step, never hand-writing journal/snapshot content): 0243_ambiguous_namor (main, unchanged) 0244_many_vermin (main, unchanged) 0245_reconcile_orphaned_github_installation_id (was 0243) 0246_salty_cyclops (was 0244) 0247_calm_lockheed (was 0245) 0248_clean_the_renegades (was 0246) 0249_groovy_agent_zero (new) Verified every regenerated file is byte-identical to its pre-renumber content (0245/0246/0247/0248 diffed clean against backups; 0247 still carries the DROP INDEX CONCURRENTLY IF EXISTS retry-safety guards and COMMIT/BEGIN fencing unchanged). 0249 is a new migration: pulled the schema-only part of feat/github-connections-pr4's commit 805e161 (github_installation_webhook_receipts.status + its CHECK constraint) forward onto this branch, since PR4 depends on it and the task asked for our five migrations to land at 0245-0249 inclusive of it. Did not pull PR4's accompanying application-logic changes (github-installations.ts, webhook-handler.ts, routers) - that is a separate feature port, out of scope for a migration renumber. schema.ts, schema.test.ts, and all four cloud-agent-next router files (apps/web/src/routers/cloud-agent-next-router.ts + .test.ts, organizations/organization-cloud-agent-next-router.ts + .test.ts) auto-merged cleanly with no conflicts; verified by inspection that both sides' changes are present (main's service-fee tables, worktree-access control/assertSessionWorktree, getMessageResult; our sharing_mode/ sharing_admission_checked_at columns, the four canonical/pending unique indexes, provider_oauth_attempts, the platform_integrations FK, and the githubAppType/platformIntegrationId router output fields). Validated per the database-migrations skill: drizzle-kit check is clean; drizzle-kit generate reports "No schema changes, nothing to migrate" (schema.ts matches the regenerated chain exactly); the full chain 0000-0249 replays successfully via pnpm drizzle:verify-bootstrap against a fresh throwaway database, and again via a direct migrate run, confirming all four unique index names, the platform_integrations_github_installation_id_..._fk (ON DELETE restrict), and both CHECK constraints (github_app_installations_sharing_mode_check, github_installation_webhook_receipts_status_check) are present with unchanged names. _journal.json has 250 entries, idx 0-249 with no gaps or duplicates. pnpm --filter web typecheck clean; oxlint clean on schema.ts; 587 tests pass across github-installations, connection-service, runtime-authorization, webhook-handler (+ dispatch integration), installation-handler, platform-integrations, both cloud-agent-next routers, schema.test.ts, review-shared-repositories, prepare-review-payload, and github-integration-helpers. pnpm format made no changes; git diff --check clean (excluding pre-existing trailing whitespace in a vendored patches/react-native@0.86.3.patch, unrelated to this merge). No conflict markers anywhere in the repository.
1 of 2 tasks
pandemicsyn
added a commit
that referenced
this pull request
Sep 16, 2026
…ns-pr4 Reconciles PR4's security hardening with the squashed PR2 (#5978) + PR3 (#6089) now in main. Eleven files conflicted; every resolution keeps main's shared-installation base and layers PR4's hardening on top (verified PR4 had no behavior that main lacked in these regions — main's conflicted hunks were the pre-hardening base): - callback/route.ts: the atomic legacy write+guard+bind transaction under lockGitHubInstallationIdentity, the NULL-safe 'col::text IS DISTINCT FROM owner.id' competing-owner guard, the verifiedIdentity gating (verified path never hits the legacy guard), and the installation_already_claimed mapping. - installation-handler.ts: the unbound-peer locked existence check (pending|active, bound or unbound, excluding the pending row and disconnected rows), the single-transaction complete+bind, and connected association resolution for suspend/unsuspend. - webhook-handler.ts: isRoutableGitHubIntegration health gating, connected tenant resolution for suspend/unsuspend, and exact association id threading into the deferred-dispatch authorization check. - github-installations.ts: the incumbent personal-owner reconnect guard, the active/disconnected sharing split, review terminalization with settle-after-commit, identity-based canonical/sibling resolution on uninstall, and the transaction-capable bind. - platform-integrations.ts: findConnectedIntegrationByInstallationId and the transaction-capable writers; connection-service keeps the best-effort all-repositories inventory sync. - github-apps-router.ts: disconnected rows excluded from installation cardinality; sharing-compatibility.ts: disconnected associations excluded from compatibility assessment. - runtime-authorization.ts needed no conflict resolution: main's reason-based API already carries the exact-id allowShared carve-out, so the generic/empty-string path stays exclusive-only. Test files are a union of both sides (all main tests retained, no duplicate test names); main's obsolete 'reconnects the same association after local disconnect and rejects another owner' is not kept because it asserts the superseded claimed_by_other_owner behavior for a disconnected incumbent, whose two scenarios PR4 covers with its split tests. Migrations are untouched: packages/db/src/migrations and schema.ts are byte-identical to main (single 0243_ambiguous_namor..0249_groovy_agent_zero).
This was referenced Sep 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Outcome
Default-off shared canonical GitHub installations for WEB Cloud Agents with exact tenant-association pinning. Slack shared use remains PR3; Code Reviewer shared routing remains PR4.
Reviewed base: current
mainatcf59d8662499749228164232615da5af0abe5134. Reviewed head:af1734d3ce0b8d11acad16622754be8618603cdb.Exact Association Boundaries
Generic token and Chat SDK webhook paths require canonical
sharing_mode = exclusive. Managed shared lookup requires an exact active association ID. GitHub bot-link signed state carries exactplatformIntegrationId; callback validates association, installation, Standard/Lite identity, canonical lifecycle, local health, and current Kilo owner or organization membership.Predeploy bot-link state without an association ID is accepted only while management and sharing admission remain disabled and only for one healthy canonical-exclusive or true-legacy literal-null app association. Explicit Standard rows require exact signed association identity. Keep features disabled for at least one signed-state TTL after deployment.
Simple Synchronous Lifecycle
Required canonical/local database cleanup runs first and fails non-2xx without recording completion. Standard bot unlink runs second as best-effort alerting. Completed-success receipts dedupe later deliveries; concurrent and crash-window repeats are idempotent. GitHub does not automatically retry failed webhooks, so manual redelivery after database failure is the accepted recovery. No lifecycle queue, cron, outbox, lease, fencing, Retry-After, or automatic retry worker exists.
Repository refresh follows lifecycle lock order and cannot revive terminal state.
Provider Reservation Boundary
Slack, Linear, and Discord OAuth attempts are durable, expiring, owner-row locked, purpose-bound, and single-use. PR3 owns provider-installation generation and lazy recovery.
Migration And Rollout
Generated migration:
0243_exotic_triton.sql. Replacement indexes build concurrently with required transaction boundaries. Before enabling sharing, stale eligible-unlinked must be zero. PR2 adds no canonical reconciliation DML.Verification
The final-rebase PostgreSQL-backed Jest/bootstrap rerun was blocked before test execution because the local OrbStack Docker daemon was unresponsive;
docker info, Compose health, andpnpm test:dbtimed out. This was not an assertion failure. The accepted pre-rebase head had the schema/bootstrap and focused web suites passing.No production queries, repairs, associations, settings, flags, backfills, deployments, or external provider operations were performed.