Enkidu Best Practices Guide
This document supplements the CODING_STYLE.md by providing architectural guidelines, security standards, and common development workflows for the Enkidu project.
1. Architecture Patterns
Service Objects (Interactions)
We aim to keep our Controllers and GraphQL Resolvers "skinny" by delegating complex business logic to Service Objects (often referred to as Interactions in this project).
When to use:
- When an action involves updating multiple models (e.g., "Create User" also sends an email and initializes settings).
- When a complex query needs to be reused.
- When you need to integrate with external APIs.
Pattern:
# app/interactions/users/create.rb
module Users
class Create < Interaction::Base
input :email, required: true
input :password, required: true
def call
user = User.new(email: email, password: password)
unless user.save
fail_with(error: user.errors.full_messages.join(', '), code: :unprocessable_entity)
return
end
SendWelcomeEmail.call(user: user)
result.details = { user: user }
end
end
endinput (not delegate_input) and fail_with (not result.fail) are the current gem API (v3.3.0) — see CLAUDE.md's Interaction Pattern section for the full example including guard.
Skinny Models
Models should primarily contain associations, scopes, and simple validations. Avoid putting complex business logic in models that makes them bloated and hard to test.
2. Security & Data Integrity
Public IDs vs. Database IDs
Never expose internal Integer IDs (primary keys) to the client.
- All public-facing models should inherit from
PublicRecord. - Use the
public_idcolumn for all API interactions and URLs. - Identify resources in controllers/resolvers using
Model.public_find(params[:id]).
Soft Deletion
We use acts_as_paranoid for most core models.
- Do not hard delete records unless strictly necessary (e.g., GDPR requests).
- Always ensure
default_scopeis respected or explicitly bypassed (with_deleted) when querying for administrative purposes.
Frontend: v-html Sanitization
Any v-html binding that renders user-supplied or dynamically constructed HTML must route through DOMPurify.sanitize() with a tight allowlist to prevent XSS.
Rule: Never bind raw HTML to v-html without sanitization.
Pattern:
import DOMPurify from 'dompurify'
// For plain HTML (e.g. bold/italic formatting):
const sanitized = DOMPurify.sanitize(userHtml, {
ALLOWED_TAGS: ['strong', 'em'],
ALLOWED_ATTR: []
})
// For SVG content:
const sanitized = DOMPurify.sanitize(svgString, {
USE_PROFILES: { svg: true, svgFilters: true }
})In the template:
<span v-html="DOMPurify.sanitize(item, { ALLOWED_TAGS: ['strong'], ALLOWED_ATTR: [] })"></span>Or compute the sanitized output in a computed property or function:
function formatItem(text: string): string {
return DOMPurify.sanitize(
text.replace(/\*\*(.+?)\*\*/g, '<strong>$1</strong>'),
{ ALLOWED_TAGS: ['strong'], ALLOWED_ATTR: [] }
)
}Reference implementations:
ChangelogModal.vue— plain HTML with<strong>tags for changelog entriesBadgeImage.vue— SVG rendering with animated badge content
Why: Unguarded v-html bindings are a common XSS vector. Even if user input is not directly involved, dynamically constructed HTML (e.g., markdown-to-HTML) can introduce unexpected tags if the transformation logic is incorrect. A tight allowlist makes the intended content explicit and prevents future contributors from copying the pattern unsanitized.
3. GraphQL Development
N+1 Query Detection
The Rails API uses Bullet to detect N+1 queries in development and test environments. In development, Bullet.raise = true crashes the server on any N+1; in test, Bullet logs N+1s but does not raise. See docs/development/n-plus-one-detection.md for how to triage and fix alerts.
Adding a New Feature
When adding a new feature that requires API exposure, follow this workflow:
- Database Migration: Create tables/columns.
- Model: implementation with specific validations and relationships.
- GraphQL Type: Create
app/graphql/types/your_model_type.rb. Expose only the fields needed by the frontend. - Resolvers/Mutations:
- For reading data: Create a resolver or add a field to
QueryType. - For modifying data: Create a mutation in
app/graphql/mutations.
- For reading data: Create a resolver or add a field to
- Frontend Integration: Update the Vue.js application to query the new fields.
N+1 Prevention
- Always be mindful of N+1 query issues in GraphQL.
- Use
includes,preload, or batch loaders (if available) when resolving associations that return collections. - A scoped aggregate on a type field (
object.assoc.where(...).count,.exists?) still re-queries per row even under anincludes()— Bullet won't flag it. Batch it in the parent resolver instead: see type-layer preload-with-fallback.
4. Testing Strategy
Unit Tests (RSpec)
- Models: extensive tests for validations and custom methods.
- Interactions: simple input/output tests. logic isolation.
- Requests/GraphQL: distinct tests for success and failure cases of mutations to ensure the API contract is met.
Frontend Tests (Vitest)
- Test components in isolation.
- Mock API responses to test state changes without hitting the real backend.
5. Performance
Database Indexes
- Ensure all foreign keys and columns frequently used in
whereclauses (likepublic_id,email,token) are indexed.
Background Jobs
- Offload long-running tasks (email sending, image processing, complex calculations) to background jobs (Sidekiq/Resque) to keep the API response time fast.
6. Gotchas & Foot-Guns
Real foot-guns caught in code review. Each entry: what went wrong → the rule → how to avoid.
Mutation re-entry guards need a synchronous check, not a reactive :disabled
A :disabled binding on the dispatching button doesn't stop a raw same-tick double dispatch — Vue's reactivity hasn't re-rendered yet when the second call fires. Every useMutation-dispatching handler needs a synchronous if (<flag>.value) return guard before the dispatch, set the flag immediately after, clear it in finally. See Mutation Re-Entry Guard for the full pattern, the shared-flag cross-blocking trap, and why a Vitest assertion alone isn't sufficient proof the guard holds.
Transaction side-effects that read the cleared column
A helper method that lazy-memoizes a lookup off a record field will silently return nil if an earlier line in the same call cleared that field inside ActiveRecord::Base.transaction. The helper reads the already-mutated in-memory record, not the pre-update value.
What hit us: Social::AcceptPartnerRequest#call ran record.update!(accountability_partner_requested_by_id: nil, …) inside a transaction, then called notify_requester, which lazily memoized @requester ||= User.find_by(id: record.accountability_partner_requested_by_id). By the time the helper ran, the column was already nil; User.find_by(id: nil) returned nil; the return unless requester guard fired; no notification was created. Fixed by capturing requester_id into a local before the transaction.
Rule: If a side-effect helper reads a column you clear inside a transaction, capture the value (or the derived record) before update!, not after. Pass it as an argument rather than relying on a memoized reader of the mutated record.
# BAD — requester lazy-reads the cleared field after update!
def call
ActiveRecord::Base.transaction do
record.update!(requested_by_id: nil, …)
end
notify_requester # @requester ||= User.find_by(id: record.requested_by_id) → nil
end
# GOOD — snapshot before the update, pass explicitly
def call
requester_id = record.requested_by_id
ActiveRecord::Base.transaction do
record.update!(requested_by_id: nil, …)
end
notify_requester(requester_id)
endRegression coverage is the spec that asserts the side-effect (expect { … }.to change { UserNotification.count }.by(1)). Keep it.
Enum renumber leaves hard-coded integer fixtures behind
When two branches collide on the same enum ordinal, the resolving merge renumbers one side (e.g. partner_missed_day: 12 → 13). Fixtures that seed notifications with a hard-coded integer (kind: 12) silently drift to the other enum value and pass their subject assertions for the wrong reason — until a dedup or scope check fails loudly several specs later.
What hit us: c12f2996 merge(OBJ-21): resolve enum conflict, partner_missed_day → 13 renumbered partner_missed_day from 12 to 13. generate_partner_missed_day_alerts_job_spec.rb:175 still seeded create(:user_notification, kind: 12, …), which is now partner_ended. The .partner_missed_day scope on UserNotification::KIND_ENUMS (rails_api/app/models/user_notification.rb:47-53) correctly skipped the seeded row, the job generated a new alert, and the dedup-guard spec expecting zero calls got one.
Rule: Prefer the symbolic kind in fixtures. kind: :partner_missed_day survives every future renumber; kind: 12 does not.
# BAD — hard-coded ordinal, invisible drift on enum renumber
create(:user_notification, user: user_b, kind: 12, acknowledged: false, …)
# GOOD — symbolic, renumber-safe
create(:user_notification, user: user_b, kind: :partner_missed_day, acknowledged: false, …)When resolving an enum collision, grep the spec suite for the numeric value (not just the symbol) before merging — symbolic references update with the rename; integer references do not.
Forcing a major bump via npm overrides can break ESM/CJS interop
When Dependabot floats a major-version bump for a transitive dependency, it's tempting to drop an overrides entry into the parent package.json and move on. The patched API surface is usually the wrong thing to look at: the consumer also has to be able to load the patched version, which depends on its module system, not its source compatibility. A patched version that's gone ESM-only will throw ERR_REQUIRE_ESM the first time a CJS consumer calls require() on it — even though npm install succeeds and source-level type checks pass.
What we caught: GHSA-w5hq-g745-h8pq landed in uuid@14, a major bump on top of v12's removal of CommonJS (ESM-only). Both alerts on our repo were transitive — docs_site via mermaid@11.14.0 (uuid 11), ionic_frontend via @cypress/request@3.0.10 (uuid 8). The first instinct was a one-line "overrides": { "uuid": "^14" } per manifest. @cypress/request@3.0.10 is CJS, so forcing uuid@^14 in ionic_frontend/package.json would have broken every Cypress run with ERR_REQUIRE_ESM the moment Cypress reached for require('uuid'). The CVE itself was unreachable in our usage (neither consumer calls v3/v5/v6 with caller-supplied buffers), so we dismissed the alerts as tolerable_risk and waited for upstream instead. See OBJ-48 for the full rationale.
Rule: Before adding an overrides entry to force a major bump, verify the consumer's module system, not just the patched version's API. If any transitive consumer is CJS and the patched version dropped CJS, the override is a runtime breakage waiting to happen — npm install won't catch it, and source-compatible API checks won't either.
// BAD — patched API surface is fine for our usage, but @cypress/request
// is CJS and uuid@12+ is ESM-only. npm install succeeds; the next
// `require('uuid')` from CJS throws ERR_REQUIRE_ESM at runtime.
{
"overrides": { "uuid": "^14" }
}
// GOOD — leave the override out. Dismiss the alert as tolerable_risk
// with rationale, and track the upstreams (mermaid, @cypress/request)
// until the CJS consumer either bumps past the breaking change or
// is replaced.Quick check before reaching for overrides:
npm why <pkg>in the manifest dir — list every consumer of the transitive dep.- For each consumer, look at its
package.json"type"(default is CJS if absent) and the file extensions in its dist (.cjsvs.mjs). - If any consumer is CJS and the patched version dropped CJS, the override is unsafe — wait for the consumer to bump, or dismiss the alert with rationale linking the upstream tracking issue.
WebView and Chrome Custom Tab have separate cookie stores in Capacitor Android
On Android, WebView and Chrome Custom Tab maintain completely separate cookie jars. When Clerk's JavaScript SDK runs in WebView and creates a session with signIn.create() or signIn.authenticateWithRedirect(), it stores a __client session cookie in WebView's cookie store. Later, when Clerk's OAuth callback URL (clerk.objectuve.com/v1/oauth_callback) is opened in a Chrome Custom Tab (a security best practice for OAuth), that Custom Tab cannot see the __client cookie — it lives only in WebView's jar. Clerk looks for the cookie to identify the pending sign-in, finds nothing, and returns {"errors":[{"code":"authorization_invalid"}]}.
This failure mode is completely invisible in local browser testing. Desktop Chrome (and mobile browser testing tools) share a single cookie context across tabs and iframe contexts, so the issue never surfaces during development.
What hit us: Two attempted fixes both failed staging because the issue was undetectable locally (OBJ-249, gsd-debug/obj249-run6). The first fix tried adding cookies to Custom Tab requests, which Chrome doesn't allow. The second attempted to pass session state via URL parameters, which Clerk's SDK doesn't support.
Rule: Use window.Clerk SDK methods via @capacitor/browser for all Clerk OAuth flows — never call Clerk FAPI endpoints directly with fetch(), axios, or @capacitor/http. The Clerk SDK builds OAuth URLs natively and negotiates auth without touching browser cookie jars, which sidesteps the WebView/Custom Tab cookie-jar split entirely.
Never use browser fetch() or axios for Clerk FAPI calls, even when running in a WebView. Both send the Origin header on cross-origin requests, which causes Clerk to switch into browser/cookie mode — the exact mode that fails in Custom Tab.
Implementation reference:
- See
src/composables/useClerkNative.ts(signInWithOAuth,completeOAuthRedirect) for the current OAuth flow
// BAD — fetch() sends Origin header, Clerk switches to cookie mode
const response = await fetch('https://api.clerk.dev/…', {
method: 'POST',
credentials: 'include'
})
// GOOD — Clerk SDK + Browser plugin avoids the cookie-jar split
import { Browser } from '@capacitor/browser'
// 1. Let Clerk build the OAuth URL natively
const signIn = await clerk.client.signIn.create({ strategy, redirectUrl, actionCompleteRedirectUrl })
const url = signIn.firstFactorVerification?.externalVerificationRedirectURL?.toString()
await Browser.open({ url, presentationStyle: 'popover' })
// 2. After the callback, complete via SDK (reads window.location redirect params)
await window.Clerk?.handleRedirectCallback({})Last updated: 2026-08-20 (PR #2528: corrected the Interaction example to the current input/fail_with API, not pre-v3 delegate_input/result.fail)