Kendo HQ

Appearance

Platform Decisions ​

Each entry is a real fork in the road. Implementation details that aren't a tradeoff are NOT here.

Feature flags — enrich ProfileResourceData, no separate endpoint ​

  • Chose: Add features: Record<string, boolean> to GET /auth/user; useFeatureActive() reads from authService.user.
  • Rejected: Dedicated GET /api/features endpoint with its own controller, route, and services/features.ts with module-level ref.
  • Why: Feature flags are per-tenant session context — same category as role, projectIds, teamIds (already on the profile). Extra request, controller, route, and service file added complexity without a concrete benefit; page reload re-fetches the user anyway.
  • Source: KD-0286

Seeders — keep behavior on missing MinIO, don't add local-disk fallback ​

  • Chose: AttachmentSeeder keeps current skip-with-warning behaviour.
  • Rejected: Switch to Storage::disk('local') when configured filesystem is unreachable.
  • Why: Masks a developer-environment problem at the seeder layer; production reads from the configured filesystem disk so local-disk attachments would be unreadable in the running app anyway. Docker is a documented hard prerequisite.
  • Source: KD-0404

Bias dense seed data toward active + completed sprints ​

  • Chose: Comments target active-sprint issues; TimeEntries target active + completed sprint issues.
  • Rejected: Even random distribution across all 50 issues.
  • Why: Realistic project-management usage concentrates this data on active work. Spreading thin produces backlog/Done noise disproportionate to where developers actually look.
  • Source: KD-0404

Lean seed volumes, reject 8× density bumps from issue body ​

  • Chose: Comments ~40, TimeEntries ~30, Notifications ~50, Reports ~15.
  • Rejected: Issue-spec targets (Comments 100-120, TimeEntries 60-80, Notifications 50-80).
  • Why: Combined with active-sprint biasing, ~40 well-placed comments produce more realistic UI than 120 randomly distributed; UIs don't pretend-paginate better at higher volumes.
  • Source: KD-0404

Audit trail — read from issue_audit_logs, no new issue_activities table ​

  • Chose: Transform existing audit-log JSON snapshots to per-field changes at read time in GetIssueActivitiesAction.
  • Rejected: New purpose-built issue_activities table with batch_id UUID grouping; OR hybrid audit + materialized view.
  • Why: Audit log already captures every issue change. A second writer hooked into every mutation duplicates data and risks drift; a materialized view adds cache-invalidation complexity for an MVP read path.
  • Source: KD-0417

Audit-trail relation resolution — server-side; enums client-side ​

  • Chose: Server resolves lane_id, assignee_id, sprint_id, epic_id to display objects (with withTrashed); client resolves priority/type enums via enumCollection.fromValue().
  • Rejected: Full server-side resolution; OR full client-side resolution from stores.
  • Why: Frontend stores don't always have deleted entities loaded — needed for historical audit entries. Enums are stable lookup tables that the client already knows about.
  • Source: KD-0417

Audit-trail tab UI — inline tab strip, not MenuTabs with queryKey ​

  • Chose: Local activeTab ref with simple flex-container styled buttons.
  • Rejected: Reuse MenuTabs shared component.
  • Why: MenuTabs is designed for router-link navigation with routes/queries — overkill for a same-page two-state toggle. ~20 lines of inline markup matches the design exactly.
  • Source: KD-0417

Audit-trail store — read-only composable, not adapter-store ​

  • Chose: Simple useActivityStore composable returning { activities, isLoading, fetch }.
  • Rejected: Adapter-store factory.
  • Why: Activities are read-only derived data — the adapter pattern's mandatory NewAdapted<T> type and create/update/delete methods would be unused.
  • Source: KD-0417

Nullable scalar coercion — three-layer enforcement, not single string-scan rule ​

  • Chose: Layer 1 (Pest DTO test) + Layer 2a (PHPStan rule) + Layer 2b (Pest heuristic).
  • Rejected: String-scan ban on forbidden patterns (!== 0 ?, !== '' ?, isset($validated[) in FormRequestsTest.php.
  • Why: String scanning can't tell nullable from non-nullable contexts; the same !== '' is a bug for nullable fields and legitimate for non-nullable ones. False positives erode trust in arch tests.
  • Source: KD-0413

Inline input() !== null ? integer() : null pattern, no helper trait ​

  • Chose: Verbose canonical pattern repeated per field.
  • Rejected: ExtractsNullable trait with nullableInt() / nullableEnum() helpers.
  • Why: PHP has no generics — nullableEnum() return type would be mixed, losing type safety. Adds scope to a bug fix; arch tests would need to recognise the helper as valid.
  • Source: KD-0413

Scalar guard scope — int only initially, expand to string/float/bool in follow-up ​

  • Chose: Phase 1 covers int only; phase 2 (KD-0477) extends coverage.
  • Rejected: Cover all nullable scalars in one pass.
  • Why: Layer 2b heuristic broadens substantially for strings (non-nullable strings are common); landing the rule for int first builds team experience with the enforcement pattern.
  • Source: KD-0413

Scalar guard — require input() !== null, ban has() ​

  • Chose: PHPStan rule strengthened to require exactly ->input(name) !== null ? ->scalar(name) : null.
  • Rejected: Accept any Ternary as a valid guard.
  • Why: has('field') is semantically wrong — it returns true when the key was present even with a null value, so the bare extraction still coerces null to 0/''/false/0.0. The correct question is whether the value is non-null.
  • Source: KD-0477

Scalar guard — fix bool = false defaults uniformly, no allow-list ​

  • Chose: LoginUserData::$rememberMe and SaveRoleData::$accessAllProjects change to ?bool = null; consuming Actions use $data->field ?? false.
  • Rejected: Allow-list these as semantically equivalent ("absent = false").
  • Why: The same "absent = default" argument applies to GlobalSearchIssuesData::$excludeFinalLane = false, which is explicitly being fixed as a bug. Case-by-case semantic carve-outs aren't enforcement at all — they are exactly the reasoning the rule prevents.
  • Source: KD-0477

Default limit belongs in the Action as a constant, not in toDto() ​

  • Chose: DTO carries ?int $limit = null; Actions define private const DEFAULT_LIMIT = 25 and apply min($data->limit ?? self::DEFAULT_LIMIT, MAX_LIMIT).
  • Rejected: Hardcode 25 in toDto() via $safe->has('limit') ? $safe->integer('limit') : 25.
  • Why: toDto() is the wrong layer for a business default; hardcoding it duplicates knowledge between the DTO and the Action's capping logic and reintroduces banned has() usage.
  • Source: KD-0477

HTTP timeouts — frontend-only scope; nginx fix tracked separately ​

  • Chose: Ship client-side timeout discipline (KD-0584); nginx fastcgi_read_timeout stays under KD-0149.
  • Rejected: One combined ticket covering frontend + nginx config.
  • Why: Blends Vue work with infra/nginx in one PR — cross-cutting review surface; KD-0149's scope explicitly anticipates a separate implementation ticket. Both halves are independent (kendo is still on fs-http 0.1.x with no client timeout).
  • Source: KD-0584

Timeout values — named constants in shared/, not typed wrapper functions ​

  • Chose: shared/constants/http-timeouts.ts re-exported from per-app services/http.ts.
  • Rejected: Per-app constants in each services/http.ts (drift risk); OR typed wrappers like uploadRequest/agentRequest (more indirection, harder to grep).
  • Why: Two apps need timeout discipline; re-export keeps app-local imports while preventing drift between tenant and central. Mirrors the existing shared/constants/icons.ts precedent.
  • Source: KD-0584

Timeout — 120s for both UPLOAD and AGENT ​

  • Chose: Match PHP's set_time_limit(120) exactly.
  • Rejected: UPLOAD 300s (generous margin); OR 60s on both (match nginx ceiling).
  • Why: Once KD-0149 raises fastcgi_read_timeout to 125s, the full chain (PHP 120s → nginx 125s → client 120s) is internally consistent. 60s would leave AI calls aborting before PHP's own deadline; 300s gives 5 minutes of "is this hung?" UX with no progress feedback for 40 MB uploads.
  • Source: KD-0584

Route hygiene epic — five small PRs, one arch test per PR ​

  • Chose: Split route-hygiene work into five sequential PRs (KD-0636 → KD-0640), each shipping one arch test green with no allowlist.
  • Rejected: One mega-PR bundling all four anti-pattern bans + meta unification; OR two PRs (hygiene then meta).
  • Why: A 600+ line auth-adjacent diff makes review fatigue and rollback granularity worse; one rule per PR means a single regression fails to a single PR and reordering stays cheap.
  • Source: KD-0636 (epic 29)

Route meta — flat object with permissions: readonly PermissionMeta[], empty array = no check ​

  • Chose: Flat RouteMeta with permissions always present; [] means "no resource check"; typed via PermissionResourceEnumValue / PermissionActionEnumValue enum-value unions.
  • Rejected: Discriminated union of {public} | {dual} | {private} with a permission: PermissionMeta | 'any' sentinel string.
  • Why: Unions and sentinel strings ('any') were explicitly disliked; empty array is the standard JS "no items" representation, TypeScript still catches missing fields, and the same flat-types-over-unions preference now applies as a generalised rule.
  • Source: KD-0636 (epic 29)

Billing access — add BILLING to PermissionResourceEnum + backfill migration ​

  • Chose: New Billing case in PermissionResourceEnum with READ + UPDATE actions and a migration inserting admin-role rows.
  • Rejected: Map billing to existing APP_SETTINGS resource; OR keep requiresAdmin: true as a third meta variant.
  • Why: Reusing APP_SETTINGS is a semantic mismatch (it gates other settings billing-viewers shouldn't see), and keeping requiresAdmin defeats the meta-unification by leaving two parallel auth concepts; a dedicated resource also unlocks tenant-level granular billing roles later.
  • Source: KD-0640 (epic 29)

Public/auth route names stay bare; arch test exempts them via inline documentation ​

  • Chose: Keep login, signup, pricing, dashboard, etc. as bare names; document the exempt list in the arch test as a comment rather than a code-level skip.
  • Rejected: Rename every public route to <domain>.<page> for a single strict arch rule with no exemptions.
  • Why: Route names are slated to seed page titles later, and auth.login reads worse as a title than login; the "allowlist" only governs which names stay bare, not which routes can bypass the factory — so the arch test still runs zero-violation against every file.
  • Source: KD-0638 (epic 29)

Route feature meta stays loose string, no typed literal union ​

  • Chose: feature?: string on metaOverrides.
  • Rejected: type FeatureFlag = 'reportTool' | 'featurePlanner' literal union.
  • Why: The source of truth for feature flags lives in GrowthBook config, not TypeScript — a TS union would drift from real config and useFeatureActive(name: string) already accepts a plain string downstream.
  • Source: KD-0636 (epic 29)

Every route declares permissions: explicitly — no factory default fallback ​

  • Chose: Force every route to declare permissions: (using [] for "no resource check"), migrating ~20 routes that previously inherited the factory default.
  • Rejected: Keep the factory default {authOnly: true, canSeeWhenLoggedIn: true} and only declare permissions when there's a real gate.
  • Why: Silent defaults re-create the bug they're meant to prevent — forgetting the field stops being a TypeScript error, and each route's access intent should be readable on the page rather than inferred from absence.
  • Source: KD-0640 (epic 29)

Tenant name uniqueness — enforce via DB unique index + form rule ​

  • Chose: Add tenant-scoped unique indexes on projects.name and teams.name, plus the missing unique rule on StoreProjectRequest.
  • Rejected: Drop the existing unique rules from update requests so duplicates are allowed everywhere.
  • Why: Uniqueness is a one-way door — relaxing it later is a one-line index drop, but imposing it on production data full of duplicates needs a conflict-resolving data migration; humans identify projects by name in standups and Slack, so duplicates feel like bugs.
  • Source: KD-0592

Column-widening migrations — group by transformation, not by table ​

  • Chose: One migration per schema transformation (varchar→text for teams; text→mediumText for issues + reports in a single closure).
  • Rejected: One migration per table (most atomic); OR one migration covering all three columns.
  • Why: Per-table maximises revertability but inflates file count past the issue body's stated "two migrations"; one-big-file loses bisectability if one tenant DB rejects one column — grouping by transformation keeps each file a single schema-level intent and matches the precedent set by KD-0585.
  • Source: KD-0593

Column-widening down() mirrors up() simply — no defensive data-length guard ​

  • Chose: ->string('description')->change() / ->text('description')->change() in down(), identical to KD-0585.
  • Rejected: Throw in down() if any row would exceed the smaller cap; OR empty down() as forward-only migration.
  • Why: A defensive row-count guard is bespoke logic no other migration does and adds query surface; empty down() diverges from Laravel + the established precedent and breaks migrate:rollback flows — rollback risk is the operator's, identical to every other column-widening migration.
  • Source: KD-0593

Description round-trip test — single combined file, one case per validation cap ​

  • Chose: Single DescriptionLengthRoundTripTest.php with four cases (project + team at 5,000 chars; issue + report at 65,535 chars), each using str_repeat('🎉', cap) 4-byte emoji.
  • Rejected: Per-domain test files mirroring KD-0585's ProjectDescriptionLengthTest.php; OR fixed-length emoji (e.g. 1,000 chars) regardless of column.
  • Why: The acceptance criterion "all four columns can store the maximum" must map row-for-row to test cases from one file, and fixed-length 4,000-byte payloads would still fit varchar(4096) — proving nothing about the actual column boundary the migration moves.
  • Source: KD-0593

Route-factory post-processing ban — line-scan + regex, not AST ​

  • Chose: Hand-rolled line-scan + paren-depth tracker in a Pest/Vitest arch spec, mirroring code-quality.spec.ts and broadcast-listeners.spec.ts.
  • Rejected: AST parser (ts-morph / typescript compiler API) walking the call-expression tree.
  • Why: No existing arch test uses AST tooling — adopting it for one rule adds a devDependency just for this check; route files are constrained enough by sibling arch tests that the regex's brittleness window stays small, and the line-scan pattern is now a reusable shape across arch specs.
  • Source: KD-0636

Planner route migration — direct createStandardRouteConfig, not widen createNestedDomainChildren ​

  • Chose: Convert plannerChildren to a direct factory call with composed name ${FEATURE_PLANNER_DOMAIN_NAME}${OVERVIEW_PAGE_NAME}, matching issueChildren / reportChildren shape.
  • Rejected: Fold a minimal createNestedDomainChildren meta-override widening into PR a so the existing nested-factory call can pass {feature: 'featurePlanner'} through.
  • Why: Widening createNestedDomainChildren is explicitly PR d's scope — bundling it into PR a breaks the "one arch test per PR" boundary and leaves PR d with nothing to do for that factory; planner only has an overview component anyway, so the nested factory was filtering 3 of its 4 outputs.
  • Source: KD-0636

Inline pricingRoute stays in router/index.ts — no dedicated domain folder ​

  • Chose: Keep pricingRoute as a factory call inside central/router/index.ts.
  • Rejected: Move it to a new pricing/route.ts domain folder.
  • Why: The arch test scans aggregators too, so a factory call there already satisfies the rule; a domain folder for one public route with no co-located pages/ directory is organisational overhead with no payoff.
  • Source: KD-0638

Export PermissionMeta from routes factory — don't hide it behind an inferred return type ​

  • Chose: Export PermissionMeta publicly when introducing createRedirectRoute.
  • Rejected: Give the new factory an explicit return type that doesn't reference the private PermissionMeta interface.
  • Why: TypeScript can't construct public re-exports without naming the interface; an explicit return type duplicates shape knowledge and PR e will refactor PermissionMeta to a typed enum-value union anyway — exporting it now lines up cleanly with the upcoming change.
  • Source: KD-0638

CI self-hosted — stop optimising at 5:12 wall-clock, leave the floor untouched ​

  • Chose: Ship the cache layer (PHPStan/Rector/Deptrac + ESLint/vue-tsc/cspell/knip) and stop; backend-feature-tests' 3:15 critical path sets a ~3:50 floor.
  • Rejected: Consolidate frontend gates into one job (10-15s); host-cache node_modules to skip npm ci (25-35s, largest single lever); cache .vite / .cache/vitest; sub-shard Test projects/issues.
  • Why: 5:12 is well under the original 15-min regression cliff, and pushing toward the floor would cost three more cache wrappers, a multi-tool job, and node_modules postinstall-script risk — readability of ci.yml's "one job per concern" shape was worth more than the remaining 80s of theoretical headroom.
  • Source: ci-self-hosted-speedups

Cap parallel-by-default tools globally, not just in CI ​

  • Chose: Commit Rector's parallel().jobs(4) in rector.php; PHPStan's --memory-limit + worker cap lives in a separate phpstan-ci.neon so local composer phpstan keeps defaults.
  • Rejected: Cap everything via CI-only config files / env vars; OR cap everything in the shared config.
  • Why: Rector's default 16 workers overloads dev laptops too — committing the cap helps both environments — but PHPStan's default fan-out is fine locally, so its CI-only override (including tmpDir: .phpstan-cache) belongs in phpstan-ci.neon to keep local invocations untouched.
  • Source: ci-self-hosted-speedups

ESLint cache uses --cache-strategy content, not mtime ​

  • Chose: eslint --cache --cache-strategy content with the cache file in a frontend-rooted path wrapped by actions/cache@v5.
  • Rejected: Default mtime-based cache strategy in node_modules/.cache/....
  • Why: npm ci wipes node_modules and actions/checkout resets mtimes on every run — mtime invalidates the cache 100% of the time, defeating the cache entirely; content-hashing survives checkout and a frontend-rooted path survives npm ci.
  • Source: ci-self-hosted-speedups

Nullable-scalar extraction — extract into a trait once the idiom hardened (reverses KD-0413) ​

  • Chose: Build the ExtractsNullableScalars trait blueprinted on the existing ExtractsIntArray trait, deduping 23 inline input() !== null ? TYPE() : null sites plus 3 copied private nullableInt() methods.
  • Rejected: Keep the verbose inline pattern per field (the position KD-0413 took when it rejected an ExtractsNullable trait), or fold the methods into ExtractsIntArray, or introduce a base FormRequest subclass.
  • Why: KD-0413's "no helper" reasoning was about type-safety on a generic helper, but the now-arch-enforced input() !== null semantics are stable enough to extract verbatim — the two PHPStan rules early-return outside toDto() and only match a ValidatedInput receiver, so a $this->-receiver trait method passes all gates while DRY-ing the duplication, and folding into ExtractsIntArray would erode its single responsibility.
  • Source: KD-0483

Eager-load contract — omit EAGER_LOAD in a subclass rather than redeclare the inherited empty default ​

  • Chose: Resources that load no relations declare nothing; static::EAGER_LOAD resolves to the base-class [] via late static binding, and from() skips the no-op loadMissing/validateRelationsLoaded calls.
  • Rejected: Declare an explicit EAGER_LOAD = [] in each subclass "for consistency" and call loadMissing([]) to match the shape of resources that do load relations.
  • Why: A subclass constant identical to the inherited value is pure noise that makes a reader wonder what was intentionally omitted — "explicit" only adds value when the content differs, so a present EAGER_LOAD should always signal meaningful entries.
  • Source: KD-0641

CI health-audit tooling — bash + jq, not a TypeScript script ​

  • Chose: Write the CI health audit as ~350 lines of bash + jq, matching the one existing repo script (check-skill-table.sh).
  • Rejected: A TypeScript (tsx) script with cleaner JSON aggregation and median/log-parse code.
  • Why: There is no root package.json, so a TS script would have to introduce a tsconfig + deps + npx tsx invocation with no offsetting test-coverage win (an ops script carries no test obligation either way), whereas bash+jq reuses gh and jq that are already present and matches the precedent the next engineer reads alongside it.
  • Source: KD-0810

Cache-health probe scopes to refs/heads/development only ​

  • Chose: Count cache entries only where ref == refs/heads/development, the shared fallback every run reads.
  • Rejected: Count entries under the prefix on any ref (including the many refs/pull/N/merge entries).
  • Why: A prefix can look healthy from PR-ref caches while the dev-ref fallback is empty — exactly the shape of the motivating Rector-empty bug, which an any-ref count would have masked.
  • Source: KD-0810

CI surfacing of the audit — separate dispatch/scheduled workflow, never gate PRs ​

  • Chose: Lint the script via shellcheck in the existing skill-table CI job, and run the audit itself only from a separate ci-health.yml (workflow_dispatch + daily schedule) that renders a job summary and fails on 🚫 only.
  • Rejected: Run the audit as a step in ci.yml on every push/PR, gating the ci-passed check.
  • Why: The audit queries historical runs (~70s + ~80 API calls) for data that changes slowly, so running it per-push is recursive, rate-limit heavy, and noisy — linting the script's correctness belongs on the cheap per-push gate, but executing the audit belongs on a cadence that never blocks a PR.
  • Source: KD-0810

Nullable-enum extraction guard — extend the existing scalar rule, not a sibling rule ​

  • Chose: Add the nullable backed-enum guard to NullableScalarExtractionRule (one new bare-enum detector, keeping the class name); enums are scalar-backed so the name stays defensible.
  • Rejected: A new NullableEnumExtractionRule registered alongside it in phpstan.neon.
  • Why: A sibling rule duplicates ~40 lines of boilerplate (toDto() scope check, FormRequest-subclass guard, DTO-constructor resolution, the type-agnostic isCorrectlyGuardedTernary() helper) — and deduping it into a shared base is more churn than the feature warrants, whereas the existing rule's already type-agnostic helper makes the enum branch ~one new detector.
  • Source: KD-0826 D1

Nullable-enum guard flags both from() and tryFrom() — the danger is the argument, not the method ​

  • Chose: Match both EnumClass::from(...) and EnumClass::tryFrom(...) when the argument is a bare ValidatedInput extraction on a nullable enum param.
  • Rejected: Exempt tryFrom because it already returns ?Enum.
  • Why: A bare $safe->integer('field') coerces an absent value to 0 before the enum sees it, so tryFrom(0) resolves to whatever case is backed by 0 (e.g. PriorityEnum::Highest) rather than null and passes composer phpstan clean — tryFrom is arguably the worse shape because it looks safe while silently landing on the most-severe case, and the argument-side check is identical for both.
  • Source: KD-0826 D3b

Nullable-enum guard requires a ValidatedInput argument — precision over recall ​

  • Chose: Flag only Enum::from(<validated-input extraction>), reusing detectBareExtractionType() on the from() argument; covers both int- and string-backed enums generically.
  • Rejected: Flag any bare Enum::from(...) on a nullable enum param regardless of argument source.
  • Why: The rule's mandate is "extraction-from-validated-input without the canonical guard," so gating on a ValidatedInput receiver mirrors the scalar rule exactly and avoids false positives on a legitimately non-null computed value assigned to a nullable param — a shape that doesn't occur and wouldn't be a regression.
  • Source: KD-0826 D3

Discovered-gap hydrator built in-scope rather than punted to a follow-up ​

  • Chose: Create the one missing ProjectTokenHydrator inside the collection-flip issue and wire both its collection and single-model call sites, amending the issue's "no new hydrators" acceptance criterion.
  • Rejected: File the gap as a separate follow-up and ship the flip around it.
  • Why: That AC assumed the prior slice had covered every loading resource — false for one — and a fully-loud collection() can't land while a single resource still self-heals, so the constraint that had to give was the AC, not the flip; the resource had no other owner to inherit the single-model wiring.
  • Source: KD-0905

Every loading collection caller routes through the hydrator, including already-safe ones ​

  • Chose: Convert call sites that were already correct via a controller-inline ->withCount(...) or an Action-held ->with(...), adding an idempotent controller-side hydrateMany while the Action's in-query load stays untouched.
  • Rejected: Leave already-safe sites alone and convert only the ones that actually self-healed.
  • Why: "Is this site safe?" is a per-site audit that decays, whereas "does the hydrator own every collection serialization site?" is one answerable rule — and the relation/aggregate guards make the redundant second load a no-op, so uniformity buys a single owner for free.
  • Source: KD-0905

Hydrator coverage is a biconditional — loading resource ⟺ hydrator exists ​

  • Chose: Gate both directions on the union of the three eager-load constants: a resource declaring any non-empty one must have a hydrator, and a hydrator must back a loading resource (no-op hydrators banned).
  • Rejected: Forward-only (loading ⇒ hydrator); OR mint a hydrator for every non-custom resource (~30 no-op classes plus tests).
  • Why: Forward-only leaves a no-op hydrator undetected, so "does this resource need a hydrator?" stops being answerable from the resource's own declaration; hydrators-everywhere pays permanent ceremony for the ~33 resources that never load. Keying on the union is what catches the resources with an empty EAGER_LOAD but a non-empty EAGER_LOAD_COUNT that an EAGER_LOAD-only check would wrongly clear.
  • Source: KD-0907 D1

Arch-test exclusions detect structurally and pin the resulting set ​

  • Chose: Exclude the 5 custom-factory resources by detecting their from()-throws pattern, plus an assertion that the detected set equals the known five.
  • Rejected: A name list of the 5 classes; OR structural detection alone.
  • Why: A name list fails open — strip the custom-factory nature from a listed class and it stays excluded, letting a real self-heal through — while pure structure is implicit and self-documents nothing; the pin turns any change to the family into a loud failure instead of a silent scope change. That makes it a structural scope boundary rather than the allowlist the issue forbids.
  • Source: KD-0907 D2

Caller gate covers both serialization surfaces — HTTP and broadcast ​

  • Chose: Gate controllers and broadcasters, widening the issue's controller-only criterion and recording the correction on the issue.
  • Rejected: Ship the HTTP-only gate as written and track the broadcast surface as a follow-up.
  • Why: A resource serializes down two caller surfaces, and the broadcast side had no total gate at all — only per-broadcaster behavioural tests plus the runtime throw — so an HTTP-only capstone half-realizes the coverage guarantee and still lets a new un-hydrated broadcaster with no behavioural test ship.
  • Source: KD-0907 D3

Gate checks hydrate-before-from() order, not just presence ​

  • Chose: Both caller gates resolve the exact bonded hydrator and require its hydrate/hydrateMany to precede the resource's first from(); inline applyTo nested in the serialization expression is inherently ordered and exempt.
  • Rejected: Presence-only static check, with the runtime validateRelationsLoaded() throw as the order backstop.
  • Why: Hydrating after from() throws at runtime — but only if a test exercises the method, which is the same "new method, no test" hole the static gate exists to close; resolving the exact bonded hydrator also catches hydrating with the wrong profile's hydrator, which a token-presence check can't see.
  • Source: KD-0907 D4

Static caller scan evaluates public entrypoints over their effective body ​

  • Chose: Scan public-method entrypoints but evaluate over the effective body — the method's own source plus the source of same-class methods it transitively calls; presence is checked over that whole body, order only when both tokens sit in one method's own body.
  • Rejected: Strict per-method scanning, either public-only (misses a from() delegated to a private helper) or all-methods (false-positives that helper, which has no hydrate token).
  • Why: One live public/private split breaks both strict variants — one goes silent on a real serialization site, the other cries wolf on correct code — and the accepted residual is narrower than either: order is simply not statically proven across a method boundary, where the runtime throw still backstops it.
  • Source: KD-0907 D4

Delete a rule that a stronger new rule subsumes ​

  • Chose: Delete the loadMissing(requiredRelations()) notation test once the new gate banned loadMissing in resource bodies outright.
  • Rejected: Leave it in place as extra coverage.
  • Why: Its premise — that loadMissing is a legal resource-body call — is now false, so it steers how to write something that can no longer exist; a strict subset of a stronger rule is misleading dead code, not defence in depth.
  • Source: KD-0907 D6

A gate that surfaces a real violation gets the violation fixed, not an exemption ​

  • Chose: Flip the one flagged site (NotificationBroadcaster::bulkUpdated) to self-hydrate at the serialization site and delete its two Actions' ->with(...) preloads, landed as its own commit ahead of the gates commit.
  • Rejected: Carve the site out of the gate so the new rule could go green on the tree as-is.
  • Why: The preloaded relations were used by nothing in those Actions — they existed solely so the downstream from() wouldn't throw, i.e. serialization knowledge leaking into a caller — so an exemption would enshrine work that had been punted between two earlier slices and dropped, and spend the gate's first allowlist entry on it. The two edits are a pair: dropping a preload is safe only because the load moved to the serialization site rather than disappearing.
  • Source: KD-0907 D9

PHPStan pseudo-types over imported generics in namespace-less files ​

  • Chose: Narrow the migration's mixed values with pluck('id')->all() + @var list<int> and a plain array_map, replacing the planned generic docblock over ->map().
  • Rejected: The planned @var \Illuminate\Support\Collection<int, int> docblock on the collection.
  • Why: The migration file has no namespace and the format-on-edit hook repeatedly stripped the matching use as unused — docblock-only references don't count as usage — leaving an unresolvable bare Collection; a PHPStan pseudo-type needs no import, so it survives the formatter instead of fighting it every edit.
  • Source: KD-0963

Rate-limiter keys — drop ?-> inside ??, don't restructure to instanceof ​

  • Chose: Rewrite user()?->id ?? ip to user()->id ?? ip at the nine authenticated limiters, with no suppression.
  • Rejected: The $user instanceof User ? $user->id : ip idiom the token-keyed limiters already use.
  • Why: PHP's ?? already reaches through a property fetch on null, so ?-> is a redundancy rule, not a null-safety claim; the instanceof bulk is earned when the key is a token id, not when ?? already expresses the fallback.
  • Source: KD-0966

PHPStan leftover — document the ignore rather than reshape vendor types ​

  • Chose: Promote the four remaining baseline entries to documented ignoreErrors (polymorphic audit-log value('hash'), abstract Mention::fromString(), Passport Client firstOrCreate values).
  • Rejected: A shared audit-log base class, or subclassing Passport's Client and rebinding it via Passport::useClientModel() so Larastan can see properties.
  • Why: Those are runtime structural changes to satisfy a static analyser, and reducing Client's $values to only the keys it $casts still errored because the vendor model ships no @property annotations.
  • Source: KD-0966

Coverage restoration — delete dead fallbacks, don't v8-ignore them ​

  • Chose: Narrow UserSettings.toggle()'s value to boolean, drop both value ?? false fallbacks, and un-exclude the five users-domain SFCs.
  • Rejected: Leave the fallbacks and mark the lines v8 ignore.
  • Why: ToggleButton emits via defineModel<boolean>(), so the undefined branch can never run; a coverage-shape lie is worse than deleting dead code.
  • Source: KD-0971

Tripwire resources — overridable construction seam, not a no-op veto hook ​

  • Chose: Move ResourceData::collection()'s body into overridable buildCollection(); the five tripwire resources throw from that override.
  • Rejected: A separate empty assertCollectible() hook, a const-driven message, unsealing collection(), or a marker interface.
  • Why: The thing being overridden is the construction itself, so the throw replaces the whole body and is size-independent; a no-op hook is two concepts plus an empty method on ~40 resources, and unsealing collection() hands every subclass the public contract so five of them can refuse it.
  • Source: KD-1012

Project-limit precheck — new minimal endpoint, not a wider billing read ​

  • Chose: GET /api/projects/limit-status returning only {project_limit, project_count}, gated on Projects:Create.
  • Rejected: Relax billing/status's manage-billing gate so project creators can read it, or ship the gate only for billing-visible users.
  • Why: BillingResourceData carries payment methods and invoices, so relaxing the gate leaks a security boundary; shipping only for admins silently degrades to "no feature" for the free-plan project creator the issue describes.
  • Source: KD-1029

Limit precheck — fail-open on fetch failure ​

  • Chose: Treat a failed limit-status fetch as "not at limit" and show the form.
  • Rejected: Fail-closed — hide the form on any fetch error.
  • Why: The backend 402 is still the real enforcement, so a failed precheck reverts to today's UX; fail-closed turns a nicety into an availability bug, including for paid tenants who were never at risk.
  • Source: KD-1029

Shallow limit-status read — inline in the controller, don't reuse the Stripe Action ​

  • Chose: Fold the three-line subscribed+count check into ProjectController::limitStatus() and extend the existing Action-exempt allowlist.
  • Rejected: Stand up a GetProjectLimitStatusAction, or reuse GetBillingStatusAction which already computes projectLimit.
  • Why: An Action whose entire body is the guard clause fails the module-shape lens (interface ≈ implementation), and reusing the billing Action would call Stripe on every create-page load for fields this endpoint does not need.
  • Source: KD-1029

Upgrade CTA — hide it when the user cannot checkout, don't widen checkout auth ​

  • Chose: A canUpgrade prop that replaces the button with "ask an admin" when the user lacks billing permission.
  • Rejected: Widen billing/checkout so Projects:Create holders can start a Stripe session.
  • Why: Who can initiate tenant checkout is a security boundary, not a UX gap; the audience this screen exists for is exactly the project creator without billing access.
  • Source: KD-1029

Error-group list — slim sibling Resource, not a conditional field or gzip ​

  • Chose: A list Resource without latest_stack_trace; the fat Resource stays for show.
  • Rejected: One Resource with a nullable/conditional trace, or leave the field and rely on gzip.
  • Why: ADR-0009 forbids branching inside a Resource, and gzip shrinks the wire but not the MySQL overflow read, PHP hydration, JSON encode, or localStorage write.
  • Source: KD-1089

Slim list — select() the columns the Resource dropped ​

  • Chose: Add an explicit column list on both list queries, kept adjacent to the Resource.
  • Rejected: Rely on the Resource alone to drop the field.
  • Why: InnoDB stores a 7 KB longText off-page, so SELECT * still follows the overflow pointer per row; dropping a JSON field is half a fix.
  • Source: KD-1089

Listing index — cover every ORDER BY column ​

  • Chose: (project_id, last_seen_at, occurrence_count) for the per-project list, and (last_seen_at, occurrence_count) for the tenant-wide list and the daily prune.
  • Rejected: The issue's two-column index (filesort remains on the tiebreaker), a bare last_seen_at (same filesort on the largest row set), or a third redundant prefix index.
  • Why: MySQL skips the filesort and can stop at LIMIT only when the index covers every ORDER BY column in order; last_seen_at is a strict prefix of the wider index, so the prune's range scan is free.
  • Source: KD-1089

Cap and index ship together ​

  • Chose: One PR, migration before the cap.
  • Rejected: Cap first, index later.
  • Why: Without the index, LIMIT 501 and uncapped plans are byte-identical — MySQL still reads and sorts every row — so a cap-only change looks like a DB fix and is only a serialization fix, which then becomes a reason to defer the index.
  • Source: KD-1089

Capped envelope — leave the adapter store ​

  • Chose: Return CappedListResourceData and give the list page a standalone fetcher holding a plain ref.
  • Rejected: Keep the adapter store, add a capped sibling and leave index uncapped, cap silently with limit(), or use retrieveInto.
  • Why: None of the nine existing capped endpoints use the adapter store (the envelope throws data.reduce is not a function); retrieveInto never evicts so pruned rows would live in localStorage forever; a {data, meta} endpoint is not an adapter-store domain.
  • Source: KD-1089

Missing optional relation — null with 200, not 404 ​

  • Chose: Return null with 200 when the issue has no originating error group.
  • Rejected: 404, or 204 No Content.
  • Why: The common case is success, not an error, and 404 would light up Nightwatch, the SPA error middleware, and the console on the normal path; 204 forces the http layer to special-case an empty body.
  • Source: KD-1089

Seat-limit fetch — fail-open with a retry banner from day one ​

  • Chose: Treat a failed fetch as "not at limit" but show a dismissible retry banner.
  • Rejected: Silent fail-open, matching the sibling feature's original plan.
  • Why: The sibling shipped silent fail-open and then grew the banner in review because the user had no signal the check had not run; build the shipped state, don't repeat the round.
  • Source: KD-1172

Limit precheck fetch — only for users who can do the action ​

  • Chose: Gate the seat-limit fetch on the same Users:Create check that shows the Invite button.
  • Rejected: Fetch on every Users-page mount.
  • Why: A Member who can view users but not invite 403s on every visit, and the retry banner treats that 403 like a network failure — permanent, unresolvable, and retrying re-403s forever.
  • Source: KD-1172

App boot — watch login, don't one-shot setup ​

  • Chose: Call boot from App.vue setup via a watch on isUserLoggedIn with {immediate: true}.
  • Rejected: A one-shot call that early-returns if logged out, or a renderless v-if child.
  • Why: App.vue stays mounted through login, so a one-shot misses /login → signed-in; a child SFC costs a component the issue did not name for a pattern echo.ts already uses on loggedInUser.
  • Source: KD-1261

App boot — stay alive during 2FA setup ​

  • Chose: Gate boot on isUserLoggedIn, not on chrome visibility.
  • Rejected: Gate on showAppChrome, which is today's behaviour.
  • Why: 2FA setup is a short overlay, not a logout; Echo is already inited whenever a user is set, and gating on chrome forces an extra refetch when 2FA finishes.
  • Source: KD-1261

Echo subscribe from a watch — wrap in effectScope() ​

  • Chose: Run start() inside an effectScope() and scope.stop() on logout (and before a second login).
  • Rejected: A new persistent-subscribe Echo API for this one caller, or switching back to a v-if child.
  • Why: subscribePrivateChannel always registers onScopeDispose, and a watch callback after login is not setup, so Vue would warn on the common login path; effectScope is already how the Echo spec stands in for a component.
  • Source: KD-1261

Transaction-hazard gates — AST, not regex ​

  • Chose: Parse with nikic/php-parser (already used by Arch and PHPStan rules) so both gates share one scanner.
  • Rejected: Regex over stripped source, the shape the audit snapshot-on-retry rule uses.
  • Why: A regex cannot see closure boundaries or the $attempts argument, so it cannot tell a delete inside the retried closure from one after it.
  • Source: KD-1334

Retry-safe delete — a new marker, not the audit-snapshot one ​

  • Chose: @retry-safe-delete: anchored to the delete call.
  • Rejected: Reuse @audit-snapshot-retry-safety.
  • Why: All ten sites the gate exists to catch already carry the old marker, so reusing it would pass the whole tree on day one while the sites stay broken; the old marker claims the audit row will not diverge, not that Eloquent's exists flag will still delete on attempt 2.
  • Source: KD-1334

Pending-fix allowlist vs permanent exclusion — two lists ​

  • Chose: A self-expiring allowlist of KD-1227 sites, plus a separately pinned non-model-receiver exclusion.
  • Rejected: One list that parks the HasMany $attachments delete with the ten pending sites.
  • Why: Self-expiry would then demand a permanently-out-of-scope site keep violating forever, and the list would stop meaning "waiting on KD-1227".
  • Source: KD-1334

Fresh-read — closed whitelist, fail closed ​

  • Chose: A delete is fresh only on three shapes: a top-level query-chain assignment, a top-level refresh()/fresh(), or a foreach whose iterable is itself a query chain.
  • Rejected: A permissive heuristic patched per review finding, or documenting the blind spots and stopping.
  • Why: "Does this variable hold a model re-read from the database" is not decidable from syntax — Collection::get shares names with the query builder, and a foreach over a captured collection is the original defect wearing a loop — so a gate should fail closed and grow the whitelist with a fixture, not loosen a predicate.
  • Source: KD-1334

Undecidable query-vs-Collection edge — state it, don't patch ​

  • Chose: Leave isQueryChain() accepting $collection->filter(...)->first(), and write the residual on the predicate.
  • Rejected: Tighten the predicate until that shape fails.
  • Why: That call chain and $tenant->subscriptions()->get() have the same syntax; telling them apart needs the receiver's type, which an Arch test cannot resolve, and tightening would start rejecting the relation chains live Actions actually use.
  • Source: KD-1334

Docs-accuracy — path-triggered reviewer, not a surface row ​

  • Chose: A docs-accuracy-reviewer spawned when the diff touches site/ or .claude/.
  • Rejected: A Row 8 on the Security & Cost Surface that the planner answers per changed text file.
  • Why: 24 of 25 mined findings were on branches that never ran /plan-feature; a shape whose findings are all on none/bug PRs is a coverage problem, and the fix is the gate, not another row.
  • Source: KD-1336

Conditional reviewer — location over closed prefixes, not meaning greps ​

  • Chose: Spawn only when git diff --name-only matches the directory prefixes site/ and .claude/ (changelog entries excluded).
  • Rejected: Always-on (exit early on empty), or a **/*.md glob.
  • Why: Meaning-greps silently skip new spellings; a directory prefix is a location git can answer, while **/*.md is not (it dropped files directly under .claude/); always-on asks an agent for findings on an empty population, the sycophancy shape already documented twice.
  • Source: KD-1336

/pr spawns the docs reviewer when the branch has no pipeline directory ​

  • Chose: /pr prompts and spawns the reviewer when there is no review artifact, and writes the verdict into the PR body.
  • Rejected: Wire it into /review-branch only, preserving /pr's "does not spawn reviewers" principle untouched.
  • Why: /review-branch's no-directory fallback writes nothing, and 24 of 25 findings were on directory-less branches — reproducing the hole the ticket closes.
  • Source: KD-1336

Compliance claims — orthogonal flag, not a second deduction ​

  • Chose: A COMPLIANCE-CLAIM flag that can sit on a SOURCED sentence and does not move the accuracy score.
  • Rejected: One score in which a legally significant sentence is just a wrong sentence.
  • Why: Scoring it twice would fail an accuracy gate on a legally significant but accurate page; the author needs "fix this sentence" and "a human has to decide" as different reports.
  • Source: KD-1336

Silent-catch rule — in the reviewed tree, not in Crit ​

  • Chose: State the rule in frontend/CLAUDE.md and Surface Row 7, identical wording, each naming the other.
  • Rejected: Put the rule in Crit's own prompt, or keep one canonical copy with a pointer.
  • Why: Crit takes no instructions from the tree it reviews but opens every CLAUDE.md as evidence, and kendo's console.* ban is a kendo decision; planner and Crit read at different times and neither follows a pointer reliably.
  • Source: KD-1337

Silent-catch — prose only, no arch test ​

  • Chose: No gate on the bare-catch shape this PR.
  • Rejected: Flag every bare catch.
  • Why: The defect is "the visible state the request was going to change is left wrong," which is not decidable from the catch block's text — half of the 23 mined findings are the rule working, so a gate would reproduce the false-positive rate it exists to remove.
  • Source: KD-1337

REVIEW_CLAUDE.md — drop it, don't keep a gitignored handoff ​

  • Chose: /review-branch is chat-only; /pr re-runs the review when this session has no verdict for current HEAD.
  • Rejected: Keep a local gitignored file so /pr can still read it after /clear.
  • Why: The file's only job was surviving /clear, and it went stale in 9 mined findings; /pr already prompts and re-runs when the handoff is missing, so the file is a stale-sha bug with no remaining job.
  • Source: KD-1357

PR body — no Review Handoff scores ​

  • Chose: Body is Summary, plus Docs accuracy / Bug Fix Verification when those gates apply.
  • Rejected: Keep the scores in the body as the only durable copy once the file is gone, with stale/refresh rules after later commits.
  • Why: Crit reviews the code, not those scores; rewriting them onto a new sha without a new review is the stale-verdict bug again.
  • Source: KD-1357

Plan files — patch only when the fix changes design ​

  • Chose: Update PLAN.md / DECISIONS.md in the same commit as a fix that changes Approach, Scope, or a Decision.
  • Rejected: Rewrite them after every /review-branch, or freeze PLAN.md after /plan-feature and only append DECISIONS.
  • Why: precedent-reviewer already fails a plan that no longer matches the diff; rewriting after every pass turns the plan into a changelog, and lint/rename fixes are not design.
  • Source: KD-1357

Final canary — reflection plus raw source, not reflection only ​

  • Chose: One test asserts ReflectionClass::isFinal() and that the file text still says final readonly class FinalCanary.
  • Rejected: Reflection only.
  • Why: A leaked BypassFinals wrapper keeps toBeFinal() green, and a reflection-only canary can be "fixed" by deleting final from the fixture while the suite stays blind.
  • Source: KD-1387

Final canary fixture — live on the BypassFinals whitelist path ​

  • Chose: Put the fixture under Fixtures/.../Actions/ so */Actions/* matches.
  • Rejected: Add */Fixtures/Kd1387/* to BypassFinals::setWhitelist.
  • Why: Expanding the mock-target list for a test fixture would let a leaked enable() strip app/Actions while this canary stayed green; the directory name is the pin.
  • Source: KD-1387

Route throttle — fix app-owned gaps, roster vendor ​

  • Chose: Give app-owned anonymous gaps a throttle; allowlist vendor, health, and local-disk serve with a reason.
  • Rejected: Throttle vendor routes we do not own, or roster the app-owned gaps.
  • Why: Throttling Passport/Cashier/MCP fights the next package update; rostering an app-owned hole would bless the thing the gate exists to close.
  • Source: KD-1388

throttleApi() — don't put it in bootstrap ​

  • Chose: Keep applying throttle:api on authenticated groups.
  • Rejected: bootstrap/app.php throttleApi(), which would wrap every api-grouped route.
  • Why: It would also throttle anonymous auth routes with the user-id-or-ip key and double-count the existing group wraps.
  • Source: KD-1388

Central throttle — share the api limiter, namespace the key ​

  • Chose: Central authenticated groups take throttle:api; the limiter keys on $user::class.':'.$user->id (IP fallback unchanged).
  • Rejected: A new central-api limiter.
  • Why: Extra named limiter for the same keying rule; without the class prefix, tenant User 5 and CentralUser 5 share one 120/min bucket.
  • Source: KD-1388

Numeric vs named throttle — named when the counter must not be shared ​

  • Chose: A named github-app-setup limiter at 60/min by IP for the setup-callback that reloads every ~6s.
  • Rejected: throttle:10,1 (alignment default) or throttle:60,1.
  • Why: Numeric throttle:N,M keys the hit counter on domain|ip alone, so raising this route's N still burns the shared counter and 429s the other 10,1 callbacks; a named limiter's key includes the limiter name.
  • Source: KD-1388

Callback throttle — before resolvers that 404-short-circuit ​

  • Chose: List throttle first on the GitHub callback routes, matching the existing webhook order.
  • Rejected: Resolver first, or changing the resolvers to always call $next.
  • Why: An invalid-state flood never consumed a limiter and still queried central; changing the resolvers is out of a route-budget PR.
  • Source: KD-1388

Named throttle: aliases — must resolve to a registered limiter ​

  • Chose: A second scan: RateLimiter::limiter(name) is callable, or the test fails with the route and the name.
  • Rejected: Prefix-only matching, or folding a missing name into the unthrottled allowlist.
  • Why: A throttle:<name> whose RateLimiter::for was never registered throws at runtime while a prefix-only gate reports the route as budgeted — the false-green this epic exists to close.
  • Source: KD-1388

Encrypted casts — explicit roster, not a name-pattern scan ​

  • Chose: Roster the eight columns that actually cast 'encrypted' today; a drop or a new encrypted column fails until someone adds it.
  • Rejected: Discover columns by /secret|token/.
  • Why: The pattern would also drag in six unencrypted opaque tokens (invite_token, remember_token, …) the issue rules out of this gate; encrypting those is a product change, not a chokepoint.
  • Source: KD-1390

Casts() scan — token_get_all, fail closed on non-literal keys ​

  • Chose: Extract the casts() body with token_get_all; when an encrypted value's key is not a string literal, fail naming the model — do not guess the column.
  • Rejected: Patch the character brace scanner to skip \', or record Model::self::API_KEY as a roster key.
  • Why: A character scan ends a string at \' and a quoted-key regex skips self::CONST => 'encrypted'; the next string form is the same hole, and guessing a column the source does not spell is a false roster entry.
  • Source: KD-1390

Fly client IP — activate the existing vhost, don't re-vendor it ​

  • Chose: ln -sf sites-enabled/default in the Dockerfile after the existing COPY, pinned by an Arch test.
  • Rejected: Vendor a new vhost file, vendor a git sites-enabled/ symlink, or keep relying on apt nginx's pre-existing symlink.
  • Why: The rewrite is already in sites-available/default; a git symlink is fragile across checkouts, and an apt package that ships without the symlink would serve the un-rewritten header.
  • Source: KD-1394

Dockerfile base — no digest pin without a bumper ​

  • Chose: Leave FROM ubuntu:22.04.
  • Rejected: Pin @sha256:… and add a dependabot docker lane.
  • Why: No docker lane exists, a digest with no bumper would rot, and the rewrite lives in the vendored vhost, not the Ubuntu package.
  • Source: KD-1394

Arch test that reads repo-root files — those paths belong on the backend CI filter ​

  • Chose: Add Dockerfile and .fly/nginx/** to the backend paths-filter.
  • Rejected: Leave them off, copying the .fly/config/*.toml hole QueueTimeoutCascadeTest already has.
  • Why: A vhost-only or Dockerfile-only PR would skip Arch Tests, so a commented-out rewrite would never run the gate.
  • Source: KD-1394

Variable-less catches — roster them, don't convert them ​

  • Chose: Allowlist each of the 12 live catch (Throwable) / catch (Exception) sites with a one-line reason.
  • Rejected: Name the variable and call report() at every site.
  • Why: Each swallow is a per-site behaviour decision, converting them is out of scope, and GuardedReport's own catch must not call report() (recursion when the log channel is what failed).
  • Source: KD-1395

@ operator — token scan, then try/catch (ErrorException) ​

  • Chose: Count the @ character token via token_get_all; replace the one real site (@getimagesize) with try { getimagesize(...) } catch (ErrorException) { false }.
  • Rejected: Regex / stripComments then @, leave @ and allowlist it, or catch Throwable.
  • Why: A text scan hits PHPDoc, emails, and heredocs; the operator is what this issue removes; Throwable would hide a real bug in the validation rule.
  • Source: KD-1395

#[CurrentUser] guard — directory-pinned, not a per-file list ​

  • Chose: Everything under Controllers/Central carries #[CurrentUser('central')]; everywhere else is bare.
  • Rejected: A per-file roster of which controllers take 'central'.
  • Why: A new Central controller would compile with a bare attribute and resolve the tenant guard until someone remembered the list; membership is the parent directory name.
  • Source: KD-1396

Auth-call scanner — ban the forms, not the file ​

  • Chose: Ban auth(, Auth::, ->guard(, and ->user( on $request / request(); a planted Socialite $driver->user() must not fire.
  • Rejected: Exempt OAuthController.php by filename, or match a six-literal list (auth()->, Auth::user, …).
  • Why: A later $request->user() in that file would go unseen, and auth('central')->user() / Auth::guard('central')->user() match none of the six literals.
  • Source: KD-1396

Empty-scan helper — two named entry points, not one flag ​

  • Chose: population / expectRoster fail on an empty scan; allowlist / expectAllowlist pass empty+empty.
  • Rejected: One function with an emptyIsFailure flag.
  • Why: Population-empty is a broken scan and allowlist-empty is a clean tree; a wrong flag turns a population gate vacuous, which is the hole this issue closes, and empty+empty cannot tell those two cases apart — the caller asserts on the files walked, never on the offender list.
  • Source: KD-1408

roster vs allowlist — the names are the two empty-cases ​

  • Chose: "roster" means population list, "allowlist" means known-offender list, matching the two entry points.
  • Rejected: lokalekeuze's single rosterDiff.
  • Why: One name for both empty-cases hides which contract the caller wanted; the four migrated scanners are allowlists, so shipping population without allowlist would leave them hand-rolled.
  • Source: KD-1408

Pre-PR finders — three lanes, not Crit's four ​

  • Chose: Keep kendo's two questions rewritten in Crit's shape, add correctness-reviewer, and fold security into runtime-integrity.
  • Rejected: Crit's four lanes verbatim, or two agents only.
  • Why: A generalist carrying the whole corpus is a fourth run on every branch for coverage the three mapped lanes already hold; two agents would leave kendo without a wrong-logic lane it has never had.
  • Source: crit-shaped-reviewers

Finders never score; the parent session synthesizes ​

  • Chose: Crit's finding schema plus a mandatory checked list; /review-branch Step 4 synthesizes in chat the way it did before the port — no verify/dedupe/waiver engine, no READY / NEEDS WORK.
  • Rejected: Crit's hunt with kendo's 0–10 score, or a skill-level copy of Crit's judge in Step 4.
  • Why: Self-reported scores cannot distinguish improvement from rubber-stamping, and a skill-level judge ports the seat deliberately left out and decides before the parent session sees the reports.
  • Source: crit-shaped-reviewers

Waived findings — file them, don't shield inside the agent ​

  • Chose: A finder files the finding and quotes the D-line in evidence; the parent session weighs it.
  • Rejected: Shield D-numbered decisions inside each agent, or make Step 4 a waiver judge.
  • Why: A suppressed finding nobody sees is the 2026-04-19 noise problem inverted.
  • Source: crit-shaped-reviewers

docs-accuracy-reviewer — delete it ​

  • Chose: Delete the agent, its path trigger, and the PR-body block; Crit's Sins of omission covers site guides, legal pages, skill docs and in-app copy on every branch.
  • Rejected: Keep it beside the new roster.
  • Why: A fourth agent whose common case is a checked-less empty report; the promised-behaviour shape is already a corpus rule.
  • Source: crit-shaped-reviewers

/pr asks for a review; a missing one does not block ​

  • Chose: No pre-PR consumer reads a Score: line or a READY / NEEDS WORK verdict; /pr asks whether to run /review-branch (default no).
  • Rejected: /pr default-yes plus a NEEDS WORK block, or only updating /review-branch and /pr while other skills still look for Score:.
  • Why: Default-yes plus a block is a demand, not an ask; leaving a skill looking for a Score: that no longer exists is a broken consumer.
  • Source: crit-shaped-reviewers

/drive-pr — one loop over CI and review, not two skills ​

  • Chose: Each cycle snapshots CI and the thread, fixes both, pushes once.
  • Rejected: One skill with two modes, or a babysit guard that refuses to push under a running review.
  • Why: A PR that is both red and reviewed still did two pushes; the win is one push per cycle, not fewer files.
  • Source: drive-pr-skill-merge

CI failures — the same five dispositions, no reply product ​

  • Chose: A red check is FIX / DESIGN CALL / FOLLOW-UP / ACCEPTED / WRONG; its product is a line in the report, not a thread reply.
  • Rejected: Drive CI green first then read findings, or add a sixth RERUN disposition.
  • Why: A pre-gate serializes a full CI cycle before a finding you could have fixed in the same push; a sixth disposition splits the table by source rather than by what you decided — babysit's "pre-existing on development" already is ACCEPTED, and a test that fails differently each iteration already is WRONG.
  • Source: drive-pr-skill-merge

Loop clock — CI only, never wait for a reviewer ​

  • Chose: Findings are read opportunistically; a verdict that lands later is picked up next cycle. Stale-head is detected from the GitHub footer, not prevented.
  • Rejected: Hold the push while a review turn is in flight, or wait once for a first verdict.
  • Why: "The reviewer is slow" and "no reviewer exists" look identical, most kendo developers do not run the harness-os daemon, and a shared-repo skill cannot differ per machine.
  • Source: drive-pr-skill-merge

Daemon probes — delete them ​

  • Chose: The skill is pure gh + git; no localhost:4748.
  • Rejected: Best-effort with --max-time 3 and a silent skip, or an opt-in flag.
  • Why: The documented "connection refused → daemon is down" never fired — 127.0.0.1:4748 black-holed the SYN with no --max-time, so every developer without harness-os hung; a shared skill should not carry a branch most of its readers cannot exercise.
  • Source: drive-pr-skill-merge

Loop budgets — two counters, not one ​

  • Chose: reviewRounds <= 3 counts cycles that disposed a reviewer finding; ciOnlyCycles <= 5 counts the rest; either exhausting stops.
  • Rejected: One budget of 3, or one budget of 5.
  • Why: Three Pint pushes would exhaust a shared 3 before a human is needed, and a shared 5 permits a fourth same-seam review round — the two numbers measure different things.
  • Source: drive-pr-skill-merge

/pr auto-invoke — stays in user memory, not the repo ​

  • Chose: Rewrite the personal memory entry that chained /pr → /babysit to /drive-pr; do not touch the repo's /pr skill.
  • Rejected: Encode the auto-invoke in the shared /pr skill.
  • Why: Encoding a personal preference in the repo would opt in every other developer — the same reason the Agent Review Requested label is not in /pr either.
  • Source: drive-pr-skill-merge

PLAN.md — no Review Notes ​

  • Chose: Drop the section; both reviewers report to the session only. Plan-time verdicts leave the calibration corpus.
  • Rejected: Keep a latest-verdict table replaced on re-run, or one **Reviewed:** line in the header.
  • Why: KD-1148 carried 769 of 1225 lines as ten rounds; a replaced verdict is ≥ 7 by construction so the audit would read nothing, and a header line proves the gate ran but nothing acts on that proof.
  • Source: lean-plan-feature-output

Plan reviewers — read-only; the planner folds findings in ​

  • Chose: Remove Edit from both reviewers; /plan-feature Phase 5 requires every finding folded into the plan body before a re-run.
  • Rejected: Reviewers apply mechanical fixes, or rewrite sections themselves.
  • Why: They run in parallel on one snapshot and would collide; a reviewer who rewrites becomes co-author and loses the independence REVIEWERS.md relies on.
  • Source: lean-plan-feature-output

Plan-review loop — two failing re-runs, then ask ​

  • Chose: After two failing re-runs the skill puts the open finding to the developer.
  • Rejected: One re-run, or no cap.
  • Why: A citation fix legitimately needs a second look; rounds three onward on KD-1148 were design disagreements, and the observed grind was five rounds per reviewer.
  • Source: lean-plan-feature-output

Plan template — cut sections with no downstream reader ​

  • Chose: Drop Planning Evidence and the testing "Mandatory Rules" / "Philosophy" boilerplate; merge Shared Reuse and Patterns to Follow into ## Reuse and patterns.
  • Rejected: Fold reuse into Key Design Decisions.
  • Why: plan-reviewer never read Planning Evidence and CLAUDE.md already states the testing rules; a cold implementer has no other way to learn which sibling to mirror, and entries not tied to a structural decision would be lost inside Key Design Decisions.
  • Source: lean-plan-feature-output

Prevention levers — size them against live reviewers ​

  • Chose: Size every lever against Crit and review-prs.
  • Rejected: Treat Town Crier's 635 findings (doc-drift, AC-unmet) as levers.
  • Why: Town Crier and prsweep no longer run; building a lever for a reviewer that has left is work nobody will see fail.
  • Source: review-finding-prevention-loop

Committed plan docs — don't stop committing them for a retired reviewer's findings ​

  • Chose: Leave PLAN/BUG/DECISIONS as they are; /pr's freshness check and /distill already own the lifecycle.
  • Rejected: Stop committing REVIEW_CLAUDE.md / BUG.md verdicts because 67 findings said a committed plan contradicted the code.
  • Why: Crit raised 3 of those 67; rewriting /pr, /review-branch and /fix-bug for a problem no live reviewer reports is not worth it.
  • Source: review-finding-prevention-loop

Known hazards — reshape the surface rows, don't add a checklist ​

  • Chose: A Client-side state Row 7, fail-open folded into Row 3, write-volume into Row 4, a Proof line on every row; hazards.md is mined evidence, not a parallel list.
  • Rejected: A repo-root checklist read by plan-feature, or one "Known hazards" row pointing at that file.
  • Why: A checklist marked N/A costs nothing and only patches the missing client-side row; surface-questions.md is already the one file the planner and the surface-reviewer share.
  • Source: review-finding-prevention-loop

runtime-integrity-reviewer — always on bug branches ​

  • Chose: /fix-bug Phase 8 spawns it beside the verifier, on every bug branch, no diff-content trigger.
  • Rejected: A trigger-grep, or leaving bug branches on bug-fix-verifier only.
  • Why: 3 of 7 stale-response findings were on bug branches; trigger-greps were retired for their silent-skip failure mode, and precedent-reviewer stays off because a bug fix has no plan prose to drift from.
  • Source: review-finding-prevention-loop

New seam — ship it with its consumers, not ahead of them ​

  • Chose: Land useLatestRequest together with its three production call sites.
  • Rejected: Ship the composable alone and migrate under a follow-up, or migrate only one site.
  • Why: src-orphan-reachability rejects a src/ module with no production importer and forbids allowlisting by design, so a seam cannot land ahead of its first consumer.
  • Source: review-finding-prevention-loop

Recurring-shape mining — repo-owned, monthly ​

  • Chose: A kendo /pr-mining skill that pulls findings for a window and proposes hazards.md rows.
  • Rejected: Crit emits a "recurring shape" note on the third hit.
  • Why: The rule lands in the repo that follows it; a crit-repo change is a later addition, not this PR.
  • Source: review-finding-prevention-loop