Kendo HQ

Appearance

Integrations Decisions ​

Distilled from 11 DECISIONS.md files. Each entry is a real fork in the road — what we chose, what we passed on, and why. Implementation details that aren't a tradeoff are NOT here (read the codebase for those).

GitHub installations data path ​

  • Chose: Embed github_installations in TenantResourceData (eager-loaded everywhere a tenant is fetched).
  • Rejected: Nested endpoint GET /central/tenants/{tenant}/github-installations, or a dedicated TenantGithubInstallationController.
  • Why: Central app has few tenants so the eager load cost is negligible. Embedding removes code (endpoint, route, controller, arch-test exemption, frontend HTTP call) rather than adding it.
  • Source: KD-0216
  • Chose: Return HTTP 409 Conflict from the backend.
  • Rejected: Change the frontend to check 422.
  • Why: 409 (conflict with current state) is the semantically correct status for "can't delete because of dependencies". 422 should be reserved for body validation errors.
  • Source: KD-0270
  • Chose: Always cascade — delete branch links + repo in one transaction; confirmation modal is the safety gate.
  • Rejected: Manual cleanup only (user removes each branch link), or force-delete with separate endpoint/param.
  • Why: Manual cleanup is impractical for repos with many branch links. Force flags add complexity for a simple action; branch links are metadata pointers, not data — Git branches remain on GitHub regardless.
  • Source: KD-0270

Billing ownership model ​

  • Chose: Hybrid — Central admins see status (read-only); tenant admins manage via Stripe Customer Portal.
  • Rejected: Central owns everything (admin-mediated), or full tenant self-service with custom UI.
  • Why: Central-owned doesn't scale (support intervention for every change). Custom self-service UI replicates Stripe's hosted Portal for no value. Stripe handles all payment UI, PCI compliance, and lifecycle management.
  • Source: KD-0344

Stripe surfaces used ​

  • Chose: Stripe Checkout + Customer Portal; keep the existing custom Pricing.vue.
  • Rejected: Add Stripe Pricing Table (replacing Pricing.vue), or build custom payment-method/cancellation UI.
  • Why: Pricing Table reduces design control. Building custom subscription management duplicates Stripe's existing UI. Hosted surfaces eliminate PCI scope.
  • Source: KD-0344

Cross-DB billing pattern ​

  • Chose: Add Cashier's Billable trait to the Tenant model (Pennant pattern); leverage existing $connection = 'central'.
  • Rejected: Enrich TenantContext on resolve (extra cross-DB query per request), or cache subscription state in Redis.
  • Why: Pennant already follows this exact pattern for feature flags — Cashier queries automatically route to central DB through the model. Per-request enrichment adds queries even when billing isn't needed; Redis adds invalidation complexity and a new infra dependency.
  • Source: KD-0344

Seat-count sync strategy ​

  • Chose: Real-time sync via a queued SyncSeatQuantityJob after user mutations commit.
  • Rejected: Periodic cron sync, or metered billing (usage at end of cycle).
  • Why: Periodic delays create disputes. Metered billing produces unexpected bills. Real-time gives accurate billing with Stripe's proration math; queueing avoids blocking user operations on Stripe API availability.
  • Source: KD-0344

Free-tier limit enforcement ​

  • Chose: Hard block — Action throws PlanLimitExceededException, frontend shows upgrade modal with Checkout link.
  • Rejected: Soft warning + grace period, or feature gating via Pennant.
  • Why: Soft enforcement is messy and hard to police. Pennant is for features, not resource limits. Hard block drives upgrade conversions and is unambiguously enforceable.
  • Source: KD-0344

Free Pro grants ​

  • Chose: Stripe 100% coupon (kendo_admin_grant, duration: forever).
  • Rejected: Local plan_override column, or Stripe indefinite trial with far-future trial_end.
  • Why: Local override splits state between DB and Stripe and hides admin grants from Stripe Dashboard. Indefinite trial abuses the trial concept and can't be distinguished from real trials. Coupons keep all state in Stripe and grants are queryable in Dashboard by coupon code.
  • Source: KD-0344

Excess data after downgrade ​

  • Chose: Excess projects/users become read-only; data is never deleted.
  • Rejected: Delete excess on downgrade.
  • Why: Upgrade should immediately restore full access. Deletion is irreversible and creates dispute risk. Read-only with clear visual indicators is the safe default.
  • Source: KD-0344

Billing plan storage ​

  • Chose: Compute BillingPlanEnum at runtime from subscription state.
  • Rejected: Store billing_plan as an enum column on tenants.
  • Why: Storage means duplication with Stripe (sync bugs) and an extra migration. Computed = single source of truth (Stripe → webhook → local subscription rows) with no drift.
  • Source: KD-0344

Billing permission model ​

  • Chose: Admin-only via is_admin; route-level middleware on backend.
  • Rejected: Add Billing to PermissionResourceEnum as the 17th resource.
  • Why: Admins already manage sensitive settings; new resource is over-engineering for now. Can always add later when granular control is needed.
  • Source: KD-0344

Duplicate git checkout button hide condition ​

  • Chose: Hide the top-level copy-branch box when branchLinks.length === 0 && !showCreateBranchForm && !showBranchSelector.
  • Rejected: Hide only when branchLinks.length > 0 (the report's literal wording), or hide only when not connected.
  • Why: The single semantic rule "show this only when no other Git UI is active and no branch is linked" eliminates the entire class of visual-duplication bugs, not just the reported case. Connection-based branching adds conditionals for a workflow already explained by the auto-link hint.
  • Source: KD-0458

GitHub App install callback shape ​

  • Chose: State param + central Setup URL — generate one-time state, pass via ?state=, verify in callback.
  • Rejected: Post-install "link to this tenant" button (webhook creates tenant_id=null, user picks afterward).
  • Why: Post-install linking adds an extra UX step; matching installations to tenants needs UX (show account login?) and is easy to get wrong. State+callback reuses the existing OAuth flow shape and ResolveGithubCallbackTenant middleware almost unchanged.
  • Source: KD-0491

GitHub App install settings location ​

  • Chose: Settings → GitHub App tab (next to AI Keys), reusing the existing tenant-admin settings home.
  • Rejected: Project settings sidebar or per-user Profile sidebar.
  • Why: Install is per-tenant, so per-project sidebar is semantically wrong (would render redundantly on every project). Profile sidebar is per-user; admin install would need extra gating to avoid showing to non-admins.
  • Source: KD-0491

GitHub App install permission ​

  • Chose: Reuse APP_SETTINGS.UPDATE (same permission as 2FA enforcement).
  • Rejected: New PermissionResourceEnum::GithubApp = 18 resource.
  • Why: User chose admins-only; new resource adds enum/policy/seed/arch-test work for no near-term benefit. If finer control is ever needed, it's a small later refactor.
  • Source: KD-0491

GitHub App disconnect semantics ​

  • Chose: Unlink only — clear tenant_id; document that uninstalling on github.com is a separate step.
  • Rejected: Unlink + call GitHub API to fully uninstall.
  • Why: Calling the install-deletion API requires an App-level JWT, is irreversible if the user just wanted a quick unlink, and is harder to test. Webhook already handles real uninstalls cleanly.
  • Source: KD-0491

Audit logging for central-DB GitHub installations ​

  • Chose: Breadcrumb only — LogManager::info(...) on link/unlink; file proper audit as KD-0539 follow-up.
  • Rejected: Reuse AppSettingAuditLogger as a proxy; build new central-DB audit infrastructure now.
  • Why: Reuse falsifies the FK and snapshot shape, breaking ADR-0001's hash-chain integrity. Building first central-DB audit infra is substantial scope (new migration, model, hash-chain logic, arch tests) for a happy-path-only v1.
  • Source: KD-0491

Cross-tenant takeover defense ​

  • Chose: Add a guard in LinkGithubInstallationToTenantAction — if installation.tenant_id is non-null and ≠ current tenant, throw installation_already_linked.
  • Rejected: Defer the case to a follow-up issue (originally classified as a happy-path edge case).
  • Why: Reachable in production after the first tenant connected: an admin on tenant A who knew tenant B's installation_id could hijack B's GitHub link via callback replay. Reclassified from edge case to security blocker before merge.
  • Source: KD-0491

Install-state token shape ​

  • Chose: Opaque Str::random(40) with payload ['tenant_id', 'user_id'] cached server-side.
  • Rejected: Original {tenantId}|{userId}|{random} encoded in the URL.
  • Why: Encoded user IDs leak through GitHub's referrer/history/proxy logs. Tenant ID is already public via subdomain but user ID is stable PII; opaque token + server-side payload prevents the leak.
  • Source: KD-0491

Defense-in-depth check after refactor ​

  • Chose: Keep the structurally-tautological $row->user_id !== $actor->id check in LinkGithubInstallationToTenantAction; rewrite the comment to spell out future-drift intent.
  • Rejected: Remove it as dead code and document the controller's row-lookup as the trust boundary.
  • Why: A future caller (queued retry with system actor, MCP tool reusing the Action with a different actor source) would silently bypass the cache→actor coupling without a tripwire. The barrier costs one if-statement; removing loses the defense.
  • Source: KD-0555

GitHub App install rate limiter ​

  • Chose: Custom github-install rate limiter at 10/min per authenticated user.
  • Rejected: 5/min per user (mirroring auth/login), or 10/min per IP.
  • Why: Auth-bypass-resilient IP keying is irrelevant once the route requires auth:sanctum,api + admin permission. 5/min risks UX friction if the admin opens multiple tabs. 10/min matches the existing story limiter shape.
  • Source: KD-0555

postMessage origin lockdown for install callback ​

  • Chose: Replace isSameBaseDomain(event.origin) with event.origin === window.location.origin locally in GithubAppSettings.vue.
  • Rejected: Add a server-issued nonce in the install URL + postMessage payload, or fix the shared isSameBaseDomain helper itself.
  • Why: Helper change needs a war-room sweep of all 3 callers before changing its contract. Nonce adds round-trip + listener state. The Blade callback runs on the same tenant subdomain as the parent, so exact-origin equality is sufficient and local.
  • Source: KD-0555

GitHub integration split — token plumbing ​

  • Chose: Add User $user to execute() signatures of GetGithubRepositoriesAction, GetGithubBranchesAction, CreateGithubBranchAction.
  • Rejected: Inject Authenticatable resolver via constructor, or inject User itself per-request.
  • Why: Resolver injection hides the user dependency and can't be tested without auth-context fixtures; it also reintroduces the container-resolved auth pattern ADR-0025 was specifically removing. Per-request User binding is a hidden global.
  • Source: KD-0630

HTTP test seam for new GitHub clients ​

  • Chose: Use Http::fake() exclusively (matching GithubAppServiceTest).
  • Rejected: Carry forward GithubServiceTest's Mockery PendingRequest chain pattern.
  • Why: Mockery chains are brittle — every method call has to be mocked in order, and adding a new chained call breaks unrelated tests. Http::fake assertions read like real API calls and survive HTTP-builder refactors.
  • Source: KD-0630

Action-body guard line shape ​

  • Chose: $token = $user->githubConnection?->access_token ?? throw new GithubNotConnectedException; — single null-coalesce-throw.
  • Rejected: Two-line instanceof check, or a requireGithubAccessToken() helper method on User.
  • Why: Helper methods hide the throw inside the model — ADR-0025's auditability rationale is specifically against that. instanceof form is verbose for a one-line concern. Class-level protected $message = 'Github not connected' was kept on the exception so feature-test response bodies stayed identical.
  • Source: KD-0630
  • Chose: Trust the existing Pusher size guard — oversize payloads drop and the client falls back to manual refresh.
  • Rejected: Trim the broadcast to the lean list shape, or cap branch links to N=10 per broadcast.
  • Why: Trimming breaks the broadcast contract (forces a refetch on every event) and a per-link cap adds a truncation contract the frontend must know about; oversize-drop is the already-documented degradation path for description-heavy issues.
  • Source: KD-0774

Resolution failure in best-effort agent tool registration ​

  • Chose: Catch \Throwable and degrade silently (no GitHub tools), logging project_id + message in the catch block.
  • Rejected: Widen the Agent deptrac allow-list to import Exceptions for a typed catch, or add a nullable tryResolve() variant on the Action.
  • Why: An architectural rule change for one catch site over-broadens the layer for future classes, and a second Action method fails the module-shape reckoning (method-per-use-case is the shallow signal); logging mitigates the over-broad catch.
  • Source: KD-0775

Repo-resolution Action signature — single throwing method ​

  • Chose: One method (Project, ?int) → ProjectGithubRepo | throws; callers needing degradation wrap in try/catch.
  • Rejected: A throwing + non-throwing (tryExecute → null) method pair.
  • Why: Adding a method per use case is the canonical shallow-and-suspect signal; keeping one surface forces callers to declare degradation intent at the catch site, and future hint dimensions extend the parameter, not the method count.
  • Source: KD-0775

Multi-repo ambiguous-resolution exception shape ​

  • Chose: One RepoSelectionRequiredException with two named factories (no-hint and hint-mismatch), carrying a primitive candidate list.
  • Rejected: A separate RepoNotFoundException for hint-miss, or reusing the existing CrossProjectException.
  • Why: Both cases render the same "pick from these N repos" UX so one class with a forking message is enough; CrossProjectException means "repo belongs to another project" and conflating it with "repo not in this project's links" muddies the audit trail.
  • Source: KD-0775

Enforcing the "use the Action, never query is_primary" convention ​

  • Chose: L1 Pest arch test that greps PHP source for query-builder reads of is_primary outside a tight two-class allow-list.
  • Rejected: An L2 PHPStan type-aware rule, or L4 code-review-only documented in CLAUDE.md.
  • Why: A custom PHPStan rule plus baseline maintenance is heavier than the gain; n=4 prior bites of this defect class is positive evidence that review-only (L4) doesn't survive without a credible trade-off.
  • Source: KD-0775

Keeping the Exceptions layer free of app-type imports ​

  • Chose: New exception extends CustomExceptionsWithErrors with primitive-only factory args; the Action pre-formats models into primitives before throwing.
  • Rejected: Widening deptrac.yaml's Exceptions: ~ to allow Models, or a side-channel wrapper service to make exceptions Models-aware.
  • Why: A pure dependency-free Exceptions layer is what makes it safe to import from every other layer, so adding Models creates circular-import-shaped risk; pre-formatting also concentrates the model→JSON translation in one site.
  • Source: KD-0775

Multi-repo branch picker layout ​

  • Chose: GroupSelect with one section per repo, branches grouped underneath.
  • Rejected: A flat SearchableSelect with owner/repo prefixed on every row, or a two-level repo-then-branch staged selection.
  • Why: Grouping matches the dimension the user actually reasons about, while prefixes repeat noisily and a two-level dropdown defeats "I know the branch but forgot the repo."
  • Source: KD-0776

Partial branch-fetch failure across repos ​

  • Chose: Promise.allSettled — render successful repos plus an inline retry error row per failed repo.
  • Rejected: All-or-nothing Promise.all (one rejection closes the picker), or silently skipping failed repos.
  • Why: All-or-nothing lets a transient 502 on one repo block linking from healthy ones, and silent skip violates the no-silent-UX-degradation guideline since the user can't tell a repo is missing.
  • Source: KD-0776

Default repo for the create-branch flow ​

  • Chose: No static default; per-project last-picked repo persisted via storageService as a Record<projectId, repoId>.
  • Rejected: Defaulting to the primary repo (is_primary), or forcing an explicit pick on every open.
  • Why: A primary-repo default keeps is_primary as a frontend correctness signal that the broader arc is retiring, while a data-derived last-picked default pre-empts that concern with zero server state.
  • Source: KD-0776

Frontend helper extraction for multi-repo fetch + storage ​

  • Chose: Keep parallel-fetch and last-picked storage logic inline in the component; no helper files.
  • Rejected: Extract fetchAllProjectBranches + lastPickedRepoStorage named helpers.
  • Why: No second consumer exists today and the shallow-test reckoning says a future consumer would extract the function rather than parameterize an inline call — so inline is right until that consumer actually appears.
  • Source: KD-0776

Repo sort order after dropping is_primary ​

  • Chose: Drop the explicit sort and rely on default insertion order (id ASC).
  • Rejected: Most-active-first via a branch_links_count aggregate, or most-recently-linked first via created_at.
  • Why: Insertion order matches today's practical default (the first-linked repo was the old primary) with no new SQL aggregate or relation load; most-active can land later as a one-line change if it proves useful.
  • Source: KD-0780

Hand-to-Claude gate on multi-repo projects ​

  • Chose: A single canHandToClaude boolean, true only when repos.length === 1; multi-repo shows a disabled button with a tooltip.
  • Rejected: The literal hasAnyGithubRepo gate (click → 422 on multi-repo), or two flags the consumer combines.
  • Why: A literal wider gate gives a worse click-to-error UX, and two flags create more surface for drift than one computed source of truth; flat refs (canHandToClaude + repoCount) are kept over a status string-union per the codebase's flat-types norm.
  • Source: KD-0780

Settings repo list after retiring the primary flag ​

  • Chose: A bare list — name + branch-link count chip + Unlink, with no replacement hierarchy marker.
  • Rejected: Replacing the "Primary" badge with a derived marker like a "First linked" badge.
  • Why: A replacement badge resurrects the very mental model the change is killing and is a UI lie, and adding a marker only to fill a visual gap is styling-driven IA without an IA justification.
  • Source: KD-0780

Retiring the is_primary arch test ​

  • Chose: Delete ProjectGithubRepoPrimaryResolutionTest.php once the column is gone.
  • Rejected: Re-scope it to ban any is_primary literal anywhere (tests, comments, docs).
  • Why: With the column removed the banned query patterns are structurally impossible (L1 → L0, a stronger guarantee), and arch tests should enforce structural rules, not grep for strings.
  • Source: KD-0780