Kendo HQ

Appearance

Platform Decisions ​

Distilled from 9 DECISIONS.md files (4 plans had no DECISIONS.md or no real tradeoffs). 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