diff --git a/.af-e2e/test-plan.json b/.af-e2e/test-plan.json index 6aaa3f8dc..2c4669081 100644 --- a/.af-e2e/test-plan.json +++ b/.af-e2e/test-plan.json @@ -34,16 +34,16 @@ "checks": [ { "id": "sdk_started", - "description": "startSDK was called", + "description": "start was called", "type": "log_contains", - "pattern": "[AF_QA][startSDK] result:", + "pattern": "[AF_QA][start] result:", "fail_action": "abort" }, { "id": "is_first_launch_true", - "description": "onInstallConversionData fires with is_first_launch=true", + "description": "registerConversionListener fires with is_first_launch=true", "type": "log_contains", - "pattern": "[AF_QA][CALLBACK][onInstallConversionData]", + "pattern": "[AF_QA][CALLBACK][registerConversionListener]", "payload_check": {"field": "is_first_launch", "expected": "true"}, "fail_action": "abort" }, @@ -65,7 +65,7 @@ "id": "get_sdk_version", "description": "getSDKVersion returns a value", "type": "log_contains", - "pattern": "[AF_QA][getSDKVersion] result:", + "pattern": "[AF_QA][getSdkVersion] result:", "fail_action": "fail" }, { @@ -106,16 +106,16 @@ }, { "id": "on_deep_linking_callback", - "description": "onDeepLinking fires (NOT_FOUND expected on clean launch)", + "description": "onDeepLinking fires (NOT_FOUND expected on clean launch) — iOS always invokes this callback on cold launch even with no deep link; Android's SDK only invokes it when an actual deep link is present, so this warns instead of fails on Android", "type": "log_contains", "pattern": "[AF_QA][CALLBACK][onDeepLinking]", - "fail_action": "fail" + "fail_action": "warn" }, { "id": "no_fatal_errors", "description": "No fatal exceptions or SDK errors in logs", "type": "absent", - "patterns": ["Fatal Exception", "FATAL", "[AF_QA][startSDK] error:", "response code:4", "response code:5"], + "patterns": ["Fatal Exception", "FATAL", "[AF_QA][start] error:", "response code:4", "response code:5"], "fail_action": "fail" } ] @@ -293,6 +293,7 @@ "scenario_ref": "E2E-005", "description": "Fresh install. Verify setCustomerUserId, setCurrencyCode, setAdditionalData propagate correctly. Identity-check event receives HTTP 200. is_first_launch=true still fires.", "requires_fresh_install": true, + "requires_device_identity_reset": true, "wait_after_launch_sec": 420, "checks": [ { @@ -326,9 +327,9 @@ }, { "id": "is_first_launch_true", - "description": "onInstallConversionData still fires with is_first_launch=true", + "description": "registerConversionListener still fires with is_first_launch=true", "type": "log_contains", - "pattern": "[AF_QA][CALLBACK][onInstallConversionData]", + "pattern": "[AF_QA][CALLBACK][registerConversionListener]", "payload_check": {"field": "is_first_launch", "expected": "true"}, "fail_action": "fail" }, @@ -352,9 +353,19 @@ "checks": [ { "id": "stop_true", - "description": "stop(true) readback present", + "description": "stop(true) resolves null — the void-RPC success value on both platforms", "type": "log_contains", - "pattern": "[AF_QA][stop] result: true", + "pattern": "[AF_QA][stop(true)] result: null", + "fail_action": "fail" + }, + { + "id": "stop_no_error_payload", + "description": "Neither stop call forwarded an RPC error through its single callback", + "type": "absent", + "patterns": [ + "[AF_QA][stop(true)] result: {\"code\"", + "[AF_QA][stop(false)] result: {\"code\"" + ], "fail_action": "fail" }, { @@ -366,9 +377,9 @@ }, { "id": "stop_false", - "description": "stop(false) readback present", + "description": "stop(false) resolves null — the void-RPC success value on both platforms", "type": "log_contains", - "pattern": "[AF_QA][stop] result: false", + "pattern": "[AF_QA][stop(false)] result: null", "fail_action": "fail" }, { diff --git a/.af-smoke/rc-test-plan.json b/.af-smoke/rc-test-plan.json index 47ec84825..bdd962a07 100644 --- a/.af-smoke/rc-test-plan.json +++ b/.af-smoke/rc-test-plan.json @@ -34,16 +34,16 @@ "checks": [ { "id": "sdk_started", - "description": "startSDK returns a result", + "description": "start returns a result", "type": "log_contains", - "pattern": "[AF_QA][startSDK] result:", + "pattern": "[AF_QA][start] result:", "fail_action": "abort" }, { "id": "conversion_data", - "description": "onInstallConversionData callback fires", + "description": "registerConversionListener callback fires", "type": "log_contains", - "pattern": "[AF_QA][CALLBACK][onInstallConversionData]", + "pattern": "[AF_QA][CALLBACK][registerConversionListener]", "fail_action": "fail" }, { @@ -139,9 +139,9 @@ "checks": [ { "id": "sdk_started", - "description": "startSDK returns a result on fresh install", + "description": "start returns a result on fresh install", "type": "log_contains", - "pattern": "[AF_QA][startSDK] result:", + "pattern": "[AF_QA][start] result:", "fail_action": "abort" }, { diff --git a/.claude/commands/release-check.md b/.claude/commands/release-check.md index d377aaeea..204a6c06e 100644 --- a/.claude/commands/release-check.md +++ b/.claude/commands/release-check.md @@ -18,7 +18,7 @@ Verify all release checkpoints. Report as a pass/fail checklist. - `package.json` version - `react-native-appsflyer.podspec` s.version - `ios/RNAppsFlyer.h` kAppsFlyerPluginVersion - - `android/.../RNAppsFlyerConstants.java` PLUGIN_VERSION + - `android/.../RNAppsFlyerConstants.kt` PLUGIN_VERSION 2. **CHANGELOG** — `CHANGELOG.md` has an entry for the current version at the top. diff --git a/.claude/commands/version-bump.md b/.claude/commands/version-bump.md index f4a5c84ff..e70ec5703 100644 --- a/.claude/commands/version-bump.md +++ b/.claude/commands/version-bump.md @@ -18,7 +18,7 @@ Bump the plugin version to `$ARGUMENTS` across all 4 files that must stay in syn 1. `package.json` — `"version": "X.Y.Z"` 2. `react-native-appsflyer.podspec` — `s.version = 'X.Y.Z'` 3. `ios/RNAppsFlyer.h` — `kAppsFlyerPluginVersion = @"X.Y.Z"` -4. `android/src/main/java/com/appsflyer/reactnative/RNAppsFlyerConstants.java` — `PLUGIN_VERSION = "X.Y.Z"` +4. `android/src/main/java/com/appsflyer/reactnative/RNAppsFlyerConstants.kt` — `PLUGIN_VERSION = "X.Y.Z"` ### Steps diff --git a/.claude/rules/bridge-patterns.md b/.claude/rules/bridge-patterns.md index a8550b9fa..6b4c7c00a 100644 --- a/.claude/rules/bridge-patterns.md +++ b/.claude/rules/bridge-patterns.md @@ -1,60 +1,100 @@ --- paths: - - "index.js" - - "index.d.ts" + - "index.ts" + - "src/NativeAppsFlyer.ts" + - "src/rn-transport.ts" --- # Bridge patterns — JS ↔ native contract -Scope: `index.js`, `index.d.ts`, and any file that calls `NativeModules.RNAppsFlyer` or `NativeModules.PCAppsFlyer`. +Scope: `index.ts`, `src/NativeAppsFlyer.ts`, `src/rn-transport.ts`. All native calls go through the single TurboModule entry point `NativeAppsFlyer.executeRpc(requestJson)` — no bespoke per-feature native methods. -## 1. Three API patterns coexist +Method dispatch, per-platform wire method-name/param resolution, and event demuxing live in `@appsflyer-sdk/js-core-plugin`, not this repo. This repo's only glue is `src/rn-transport.ts`'s `RNTransport`, implementing `RpcTransport`: -| Pattern | When used | Detection | -|---------|-----------|-----------| -| Dual callback/promise | `initSdk`, `logEvent` | `if (success && error)` routes to `*WithCallBack`; otherwise `*WithPromise` | -| Callback-only | Most config methods (`setCustomerUserId`, `stop`, `setCurrencyCode`) | Optional callback; defaults to `console.log` fallback | -| Event emitter | Deep linking, conversion data, purchase validation | `appsFlyerEventEmitter.addListener(eventName, handler)` | +| `RpcTransport` member | Implementation | +|---|---| +| `call(method, params)` | Serializes to `executeRpc`'s request JSON, parses the response, resolves with `data` or rejects with `error` | +| `subscribe(listener)` | Wraps `NativeEventEmitter` on the shared `RNAppsFlyer_rpcEvent` event name | -When adding a new method, match the pattern of similar methods. Do not mix patterns within a single method. +`index.ts` constructs `AppsFlyerSDK` with an `RNTransport` instance, exports it (`export const AppsFlyer = sdk`), and re-exports everything from `@appsflyer-sdk/js-core-plugin` (`export * from "@appsflyer-sdk/js-core-plugin"`). -## 2. Callback-to-native routing +## RPC request/response shape -```js -// Dual pattern — index.js -if (success && error) { - RNAppsFlyer.initSdkWithCallBack(options, success, error); -} else { - return RNAppsFlyer.initSdkWithPromise(options); -} +Every call serializes to: +```json +{ "method": "methodName", "params": { ... } } +``` + +Every response resolves (never rejects for native-side outcomes) as: +```json +{ "success": true, "data": } +// or +{ "success": false, "error": { "code": , "message": "" } } +``` + +`RNTransport.call` unwraps this: resolves with `data` on success, rejects with `error` on failure. Android's `error.code` is a distinct number per failure class (e.g. `404` = `RpcErrorCodes.METHOD_NOT_FOUND`) — iOS error codes aren't cross-checked against this numbering, don't assume parity. + +The TurboModule Promise itself only rejects if the call never reaches native at all. + +## Event channel contract + +Async native events (conversion data, deep link, session ready) arrive via `NativeEventEmitter` on a single shared event name (`RNAppsFlyer_rpcEvent` on both platforms). `RNTransport.subscribe` forwards the raw envelope to `@appsflyer-sdk/js-core-plugin`, which owns the demuxing into: +- `onConversionDataSuccess` / `onConversionDataFail` +- `onDeepLinkReceived` (iOS) / `onDeepLinking` (Android) — normalized to one JS-facing shape +- `onSessionReady` — fires once `registerSessionReadyListener` is registered and native signals readiness. `isSessionReady` is a separate one-off Promise query for current state, not a replacement. + +The raw `origin`/`timestamp` envelope fields are stripped before handing `data` to app callbacks. No `supportedEvents` array under TurboModules. + +## Listener registration order + +Registration calls are init-order-independent by design on both platforms — each just assigns a delegate/callback on the persistent native SDK singleton. Only `start`/`logEvent` require `init` to have run first. + +Exceptions (native side effects — see `known-issues-kb.md` for root cause): + +| Listener | Call order | Why | +|---|---|---| +| `registerSessionReadyListener` | called synchronously (no `.then()`) | avoids a TOCTOU race with `init()` | +| `registerDeepLinkListener` | before `init()`, both platforms | Android drops any pre-registration deep-link result with zero buffering | +| `registerConversionListener` | either order | plain delegate assignment, no side effect | + +### Canonical call order + +``` +registerDeepLinkListener → init → (config setters) → registerConversionListener → registerSessionReadyListener(() => start()) ``` -The native side has **separate methods** for callback vs promise variants. Adding a new dual method requires implementing both on iOS (`RCT_EXPORT_METHOD`) and Android (`@ReactMethod`). +Register synchronously, not inside `init(...).then()` — deferring into the promise callback delays the one callback that triggers `start()`. -## 3. Event emitter contract +No JS-side buffer exists for these RPCs — don't add one without confirming an actual native regression first (see `known-issues-kb.md`'s "listener-registration buffer removed" entry). -- Events arrive as **JSON strings** from native — always parsed with `JSON.parse` on the JS side -- Parse failures produce `AFParseJSONException` objects (not proper Error subclasses) -- Native must serialize data to JSON string **before** calling `sendEventWithName:body:` (iOS) or `sendEvent` (Android) -- Supported event names are declared in iOS `supportedEvents` and must match exactly on both platforms: - `onAttributionFailure`, `onAppOpenAttribution`, `onInstallConversionFailure`, `onInstallConversionDataLoaded`, `onDeepLinking`, `onValidationResult` +### Deterministic ordering after start() -## 4. Listener registration order +`registerSessionReadyListener`'s callback is the only place to call `start()`. It fires asynchronously, so code written after the registration call in source order runs *before* it, not after. Wrap in a Promise to run app logic strictly after `start()`: -`onDeepLink` (and `onInstallConversionData`, `onAppOpenAttribution`) must be registered **before** `initSdk`. The native SDK fires these callbacks immediately after initialization — if the JS listener isn't attached yet, events are lost silently. +```js +function startWhenSessionReady() { + return new Promise((resolve, reject) => { + AppsFlyer.registerSessionReadyListener(() => { + AppsFlyer.start().then(resolve, reject); + }); + }); +} +// ... init() + listener registration (not awaited) ... +await startWhenSessionReady(); +``` -This is the #1 source of GitHub issues (#650, #647, #630, #305, #292). Always validate listener timing in code review. +This only reorders app-controlled code — there's no timeout/fallback if `onSessionReady` never fires. -## 5. No transpilation +`onAppOpenAttribution`, `onAttributionFailure`, `performOnAppAttribution` are removed in 7.0.0 — use `onDeepLinking` instead (see MIGRATION.md). -`index.js` ships as-is via npm — no Babel, no bundler. Write only syntax that Metro and Node can consume directly. The file uses ES module `export` syntax with CommonJS-compatible patterns. +## No transpilation -## 6. Named exports +`index.ts` ships as-is via npm — no Babel, no bundler, no separate `index.js`/`index.d.ts`. Only syntax Metro and Node can consume directly. -Current named exports from `index.js`: `AppsFlyerConsent`, `AFParseJSONException`, `AFPurchaseType`, `MEDIATION_NETWORK`, `StoreKitVersion`, `AppsFlyerPurchaseConnector`, `AppsFlyerPurchaseConnectorConfig`. +## Named exports -Adding a new named export changes the public API surface — requires a minor version bump and matching `index.d.ts` update. +`AFInAppEventType`, `AFPurchaseType`, `MEDIATION_NETWORK`, `StoreKitVersion`, `AppsFlyerPurchaseConnector`, `AppsFlyerPurchaseConnectorConfig`, plus everything `@appsflyer-sdk/js-core-plugin` exports (including `AppsFlyerConsent`). Adding a new named export requires a version bump. -## 7. Default callback fallback +## PurchaseConnector -Many methods use `(result) => console.log(result)` as the default callback when none is provided. This leaks to production logs. Prefer silent no-ops for new methods, or document the logging behavior explicitly. +`PCAppsFlyer` still uses the legacy `NativeModules` bridge — out of scope for the TurboModule rewrite. Don't touch `PurchaseConnector/` when working on RPC/TurboModule changes. diff --git a/.claude/rules/expo-config.md b/.claude/rules/expo-config.md index 1bee1a1e7..62dab9668 100644 --- a/.claude/rules/expo-config.md +++ b/.claude/rules/expo-config.md @@ -5,56 +5,43 @@ paths: # Expo config plugin rules -Scope: `expo/` directory — `withAppsFlyer.js`, `withAppsFlyerIos.js`, `withAppsFlyerAndroid.js`. +Scope: `expo/withAppsFlyer.js`, `withAppsFlyerIos.js`, `withAppsFlyerAndroid.js`. These run at `expo prebuild` time to modify native project files; the host app must have New Architecture enabled (not enforced by the plugin itself). -## 1. Config plugin structure +## Structure ``` expo/ ├── withAppsFlyer.js ← Entry point, composes iOS + Android plugins -├── withAppsFlyerIos.js ← Modifies AppDelegate for deep link handling -├── withAppsFlyerAndroid.js ← Modifies AndroidManifest.xml -└── withAppsFlyerAppDelegate.js ← AppDelegate code injection +├── withAppsFlyerIos.js ← Modifies AppDelegate (ObjC + Swift) + Podfile +└── withAppsFlyerAndroid.js ← Modifies AndroidManifest.xml ``` -These are Expo Config Plugins — they run at `expo prebuild` time to modify native project files. +## Swift AppDelegate support -## 2. Swift AppDelegate problem (critical, unresolved) +`withAppsFlyerIos.js`'s `modifySwiftAppDelegate` string-matches Expo's default Swift AppDelegate template (`didFinishLaunchingWithOptions`/`openURL`/`continueUserActivity`) and injects `handleLaunchOptions`/`handleOpen`/`continueUserActivity` calls via `AppsFlyerAttribution.shared` (one `import react_native_appsflyer`, no `AppsFlyerLib` import needed). `modifyObjcAppDelegate` handles the legacy ObjC template the same way. -Starting with Expo SDK 52 / RN 0.76, the default AppDelegate is **Swift** (not Objective-C). The plugin's `withAppsFlyerAppDelegate.js` modifies ObjC code and **fails silently** on Swift AppDelegates (#638, #620). +Both matchers are exact-string-match against one template shape — if Expo/RN changes the default AppDelegate boilerplate, the matcher silently misses (falls through to `WarningAggregator.addWarningIOS`, not a build failure) instead of adapting. Re-verify the matched strings against a fresh `expo prebuild` output whenever bumping the supported Expo SDK version. -Until this is fixed: -- Do not assume AppDelegate is ObjC in config plugin code -- Test with both `expo prebuild` (Swift default) and legacy ObjC projects -- This is the #1 Expo compatibility blocker +## Manifest merge is not idempotent -## 3. Manifest merge duplication +`withAppsFlyerAndroid.js` appends `tools:replace` entries to `AndroidManifest.xml` without checking for existing entries — repeated `expo prebuild` (without `--clean`) duplicates them and breaks the Android build. Always check if the entry exists before appending. -`withAppsFlyerAndroid.js` appends `tools:replace` entries to `AndroidManifest.xml`. Running `expo prebuild` multiple times (without `--clean`) causes **duplicate entries** that break the Android build (#672). +## Expo Go incompatibility -Fix pattern: always check if the entry exists before appending. Use idempotent modifications. +Requires native modules unavailable in Expo Go — only works in development builds (`eas build --profile development`) or bare workflow. -## 4. Expo Go incompatibility +## No test coverage -The plugin requires native modules unavailable in Expo Go. Only works in development builds (`eas build --profile development`) or bare workflow. This is documented but users miss it repeatedly (#542). +Zero test coverage on the config plugins. Manual test with `expo prebuild --clean` on both platforms after any change. -## 5. No test coverage +## Peer dependency -The Expo config plugins have **zero test coverage**. When modifying these files, manual testing with `expo prebuild --clean` on both platforms is required. Consider adding unit tests that mock the Expo config plugin API. +`expo` is an optional peer dependency — guard all Expo-specific imports/config so the plugin works without Expo installed. -## 6. Peer dependency - -`expo` is declared as an optional peer dependency. The plugin must work without Expo installed — guard all Expo-specific imports and config. - -## 7. Testing changes +## Testing changes ```bash -# Clean prebuild (recommended) cd demos/demo && npx expo prebuild --clean - -# Verify Android manifest cat android/app/src/main/AndroidManifest.xml | grep -A5 "appsflyer" - -# Verify iOS AppDelegate -cat ios/demo/AppDelegate.m # or AppDelegate.swift for Expo 52+ +cat ios/demo/AppDelegate.swift # or AppDelegate.m pre-Expo-52 ``` diff --git a/.claude/rules/known-issues-kb.md b/.claude/rules/known-issues-kb.md index 787ed0b04..0b9c8be09 100644 --- a/.claude/rules/known-issues-kb.md +++ b/.claude/rules/known-issues-kb.md @@ -1,139 +1,118 @@ # Known issues knowledge base -Issue-based KB derived from real GitHub issues. Reference when debugging user reports, reviewing PRs, or adding new features. +Issue-based KB derived from real GitHub issues. Reference when debugging user reports, reviewing PRs, or adding new features. Resolved issues are kept only where the root cause explains a non-obvious current constraint — otherwise they're cut once fixed. -## Deep linking (62 issues — #1 category) +## Bridge architecture: no listener-registration buffer + +Both native bridges used to hold `init`/listener-registration RPCs in a JS-side queue until `init` resolved, on the assumption native silently drops early registrations. That assumption was wrong: registration just assigns a delegate/callback on the persistent native SDK singleton, confirmed against native RPC source on both platforms — it's init-order-independent by design. The buffer was removed on both platforms. + +**Do not re-add a buffer/gate on either platform** without first confirming an actual native regression. `bridge-patterns.md`, `native-ios.md`, and `native-android.md` all point here instead of re-explaining this. + +**Real exception** (native-side effect, not a buffering problem): Android `registerDeepLinkListener`'s pre-init requirement, below. + +## Deep linking ### Listener not firing -**Issues:** #650, #647, #630, #305, #292 -**Root cause:** `onDeepLink` registered after `initSdk`, or native AppDelegate/MainActivity setup missing. -**Fix:** Register listeners before `initSdk`. Verify `continueUserActivity`/`openURL` in AppDelegate, intent filters in AndroidManifest. -**Test:** Killed state → open deep link → verify callback fires within 5s. +**Root cause:** `onDeepLink`/`registerDeepLinkListener` registered after `init`, or native AppDelegate/MainActivity setup missing. +**Fix:** Register listeners before `init`. Verify `continueUserActivity`/`openURL` in AppDelegate, intent filters in AndroidManifest. ### Deferred deep link not working -**Issues:** #650 (Android), #305 (iOS) -**Root cause:** Conversion data round-trip is slow or fails. No "completed with no result" callback. -**Fix:** Use `onInstallConversionData` as fallback. Check `is_first_launch` flag. +**Root cause:** Conversion data round-trip is slow or fails; no "completed with no result" callback. +**Fix:** Use `onInstallConversionData` as fallback. Check `is_first_launch`. ### Inconsistent payload shape -**Issues:** #292, #242 **Root cause:** Android returns stringified JSON where iOS returns an object in some versions. -**Fix:** Always `JSON.parse` if typeof is string. Type definitions should reflect the union. +**Fix:** Always `JSON.parse` if `typeof` is string. Type definitions should reflect the union. -## iOS build failures (22 issues) +### Android `registerDeepLinkListener` must be called before `init()` — still true, no native buffering +**Root cause:** `AFDeepLinkManager`'s `onDeepLinking()`/`onDeepLinkingSuccess()`/`onDeepLinkingError()` guard on `if (listener != null)` with zero buffering — a result arriving while `listener` is still null is dropped permanently. Real constraint, not folklore. +**Fix:** Register before `init()`, both platforms (canonical order in `bridge-patterns.md`). -### Header not found -**Issues:** #633 (`AppsFlyerConsent.h`), #602 (`AppsFlyerAdRevenueData.h`), #646 (`react_native_appsflyer-Swift.h`) -**Root cause:** Podspec pins native SDK version; cached pods have stale headers. -**Fix:** `pod deintegrate && pod install --repo-update`. Match plugin version to compatible native SDK. +### `registerDeepLinkListener` fabricates `status: 'NOT_FOUND'` on payloads with no status field +**Root cause:** `@appsflyer-sdk/js-core-plugin`'s `normalizeDeepLinkStatus` defaults any unrecognized/missing status to `'NOT_FOUND'`, including legacy `onAppOpenAttribution`-merged payloads that never had a status field. +**Not fixable from this repo** — real npm dependency, no interception point. Tests assert the dependency's actual behavior (`{...payload, status: 'NOT_FOUND'}`) rather than raw pass-through. +**Long-term fix:** needs to ship upstream in js-core-plugin. + +### Android deferred deep link delivers `status: 'FOUND'` with an always-empty `deepLink: {}` +**Root cause:** native sends `deepLink.clickEvent.toString()` as real JSON, but js-core-plugin's `normalizeDeepLinkPayload` parses it assuming Java's `Map.toString()` format (`key=value` pairs). JSON has no `=`, so every field fails to parse and the function returns `{}` unconditionally. +**Why direct/warm-start opens look fine:** apps typically read the Intent URL directly via `Linking` on those paths, never touching this field — fresh install has no Intent URL to fall back on, so it's fully exposed. +**Not fixable from this repo** — real npm dependency. **Long-term fix:** js-core-plugin needs a `JSON.parse()` branch for the actual `af-android-plugin-bridge` 7.0.12+ wire format. + +## iOS build failures -### Symbol collision -**Issues:** #497, #541 (redefinition of `SUCCESS`) -**Root cause:** Native SDK enum name collides with other libraries. -**Fix:** Upgrade to plugin version where enum was namespaced. +### Header not found +**Root cause:** either stale cached pod headers after a version bump, or mixed Swift/ObjC without the bridging header wired up. +**Fix:** `pod deintegrate && pod install --repo-update` for stale headers; verify `RNAppsFlyer-Bridging-Header.h` is set in Xcode build settings for the bridging-header case. -## Android build failures (13 issues) +## Android build failures ### Namespace not specified -**Issues:** #583, #561 -**Root cause:** AGP 8.0+ requires `namespace` in build.gradle. Plugin pre-6.15.1 lacks it. -**Fix:** Upgrade plugin to 6.15.1+. +**Root cause:** AGP 8.0+ requires `namespace` in `build.gradle`. +**Fix:** upgrade plugin to 6.15.1+. ### AndroidManifest merge conflicts -**Issues:** #627, #631 -**Root cause:** Plugin manifest declares `tools:replace` that conflicts with other libraries. -**Fix:** Add explicit `tools:replace` in app's main AndroidManifest.xml. +**Root cause:** plugin manifest declares `tools:replace` that conflicts with other libraries. +**Fix:** add explicit `tools:replace` in the app's main AndroidManifest.xml. -## Native module null / not found (13 issues) +## Native module null / not found ### RNAppsFlyer is null -**Issues:** #587, #401, #174, #333 -**Root cause:** Autolinking not triggered after install, or New Architecture enabled with old plugin version. -**Fix:** Run `pod install` (iOS) / Gradle sync (Android). For New Architecture: upgrade to 6.15.1+. Restart Metro: `npx react-native start --reset-cache`. - -## Expo compatibility (18 issues) +**Root cause:** autolinking not triggered after install, or New Architecture enabled with an old plugin version. +**Fix:** `pod install` (iOS) / Gradle sync (Android); upgrade to 6.15.1+ for New Architecture; restart Metro with `--reset-cache`. -### Swift AppDelegate not supported -**Issues:** #638, #620 -**Root cause:** Config plugin only modifies ObjC AppDelegate. Expo 52+ defaults to Swift. -**Fix:** Pending upstream fix. Workaround: manual native setup. +## Expo compatibility ### Duplicate manifest entries -**Issues:** #672 -**Root cause:** `withAppsFlyerAndroid.js` not idempotent. -**Fix:** Use `expo prebuild --clean` (not just `expo prebuild`). +**Root cause:** `withAppsFlyerAndroid.js` is not idempotent. +**Fix:** use `expo prebuild --clean`, not plain `expo prebuild`. -## Runtime crashes (18 issues) +## Runtime crashes ### Double callback invocation -**Issues:** #601 -**Root cause:** Native bridge calls JS callback more than once. -**Fix:** `CallbackGuard` added in 6.17.8 (Android). Every new callback method must use it. +**Root cause:** native bridge calls the JS callback more than once. +**Fix:** `CallbackGuard` on Android — every new callback method must use it (legacy bridge only, see `native-android.md`). ### ConcurrentModificationException -**Issues:** #447 -**Root cause:** Thread safety issue in native Android SDK. -**Fix:** Upgrade native SDK to patched version. +**Root cause:** thread-safety issue in the native Android SDK. +**Fix:** upgrade native SDK to a patched version. -## Event tracking / logEvent (13 issues) +### Android session-ready can stall if `init()` runs before the host Activity's first `onResume` +**Root cause:** `AndroidLifecycleManagerImpl` only replays a missed `onActivityResumed` transition when the context passed to `init()` is literally an `Activity`. `RNAppsFlyerModule.kt` supplies `{ reactApplicationContext.currentActivity ?: reactApplicationContext }` (lazy, fresh per call) to cover the normal case, but if `init()` dispatches before any Activity has resumed, this falls back to `reactApplicationContext` and the stall can still occur. +**Not expected on RN's normal launch path; no contract test yet for this fallback.** + +## Event tracking / logEvent ### 404 on logEvent -**Issues:** #491, #390 -**Root cause:** Wrong `appId` on Android (should be package name or omitted, not iOS App Store ID). -**Fix:** Use `Platform.select()` for `appId`. On Android: omit or use package name. +**Root cause:** wrong `appId` on Android (should be package name or omitted, not the iOS App Store ID). +**Fix:** `Platform.select()` for `appId`; omit or use package name on Android. ### "no devKey" error -**Issues:** #645 -**Root cause:** `logEvent` called before `initSdk` completes. -**Fix:** Await `initSdk` resolution before calling `logEvent`. +**Root cause:** `logEvent` called before `init` completes. +**Fix:** await `init` resolution before calling `logEvent`. -### logEvent callback never fires on Android (CallbackGuard WeakReference) -**Issues:** discovered in E2E testing (2026-05-12) -**Root cause:** `CallbackGuard` (added in 6.17.8) wraps `Callback` in `WeakReference`. All other methods invoke callbacks synchronously before the `@ReactMethod` returns, so the strong reference on the call stack keeps them alive. `logEvent` is the only method where the callback fires asynchronously — `AppsFlyerRequestListener.onSuccess()` runs on a background thread ~2s later after the HTTP round-trip. By then, GC has collected the weakly-referenced `Callback`. -**Symptoms:** Native SDK sends events successfully (200 OK in logcat), but JS success/error callbacks are silently swallowed. No error logged. -**Fix:** Use the Promise-based API (`logEvent(name, values)` without callbacks → returns Promise) which uses `Promise` instead of `Callback`. `Promise` is held strongly by the bridge and is not affected. -**Long-term fix:** `CallbackGuard` should use a strong reference for async callbacks, or `logEvent` should keep a strong reference alongside the `WeakReference`. +### logEvent callback never fires on Android (legacy `CallbackGuard`) +**Root cause:** `CallbackGuard` wraps `Callback` in a `WeakReference`. Every other method invokes its callback synchronously (keeping it alive via the call stack), but `logEvent`'s callback fires ~2s later on a background thread after GC has already collected it. +**Fix:** use the Promise-based `logEvent` API — Promises are held strongly by the bridge and unaffected. -## Privacy / ATT / compliance (20 issues) +## Privacy / ATT / compliance ### ITMS-91064 App Store rejection -**Issues:** #673 -**Root cause:** `static_framework = true` places PrivacyInfo.xcprivacy where Apple's tooling doesn't scan. -**Fix:** Use dynamic linking (`static_framework = false`). +**Root cause:** `static_framework = true` places `PrivacyInfo.xcprivacy` where Apple's tooling doesn't scan it. +**Fix:** use dynamic linking (`static_framework = false`). ### ATT popup not showing -**Issues:** #328, #619 -**Root cause:** `waitForATTUserAuthorization` must be set before `start()`. User must be prompted first. -**Fix:** Call `requestTrackingAuthorization` before `initSdk`, set timeout value. +**Root cause:** ATT authorization must be requested and resolved before `start()`. +**Fix:** call `requestTrackingAuthorization` before `init`, with a timeout. ### Android AD_ID permission -**Issues:** #593, #562 **Root cause:** Google Play requires explicit `AD_ID` permission declaration. -**Fix:** Add `` to app manifest. - -## TypeScript types (11 issues) - -### Types don't match runtime -**Issues:** #670, #575, #475, #194 -**Root cause:** `index.d.ts` is hand-maintained and drifts from actual native output. -**Fix:** Verify types against native output on both platforms. Use `patch-package` as user workaround. - -## RN version compatibility (13 issues) - -### podspecPath / config.js invalid -**Issues:** #458, #421, #403, #395 -**Root cause:** RN 0.68+ changed `react-native.config.js` schema. -**Fix:** Upgrade plugin to version matching RN version. +**Fix:** add `` to the app manifest. -### NativeEventEmitter warning -**Issues:** #335 -**Root cause:** RN 0.65+ requires `addListener`/`removeListeners` on native modules. -**Fix:** Upgrade to plugin version with stub methods. +## RN version compatibility -### Event callbacks silent with local path dependency (file:..) -**Issues:** SO#79083213, discovered during E2E 2026-05-12 -**Root cause:** When the plugin is referenced via `"file:.."` in `package.json` (local development), both the plugin root and the example app get their own `node_modules/react-native`. The plugin's `index.js` creates a `NativeEventEmitter` from its copy, while the app runtime uses the example's copy — two separate event bus instances. All event callbacks (`onDeepLink`, `onInstallConversionData`, `onAppOpenAttribution`) silently fail because listeners register on bus A while native emits on bus B. -**Fix:** In the example/demo app's `metro.config.js`, add `extraNodeModules` to force all `react-native` imports to resolve from the example's `node_modules`, and `blockList` to prevent Metro from resolving the parent's copy: +### Event callbacks silent with local path dependency (`file:..`) +**Root cause:** with a `"file:.."` dependency, the plugin and the app get separate `node_modules/react-native` copies — `src/rn-transport.ts` builds its `NativeEventEmitter` from one copy while the app runtime uses the other, so listeners register on one event bus while native emits on the other. +**Fix:** in the app's `metro.config.js`, force `react-native`/`react` to resolve from the app's own `node_modules` via `extraNodeModules`, and `blockList` the plugin's copies: ```js extraNodeModules: { 'react-native': path.resolve(__dirname, 'node_modules/react-native'), @@ -144,4 +123,4 @@ blockList: [ new RegExp(path.resolve(pluginRoot, 'node_modules/react').replace(/[/\\]/g, '[/\\\\]') + '[/\\\\].*'), ], ``` -**Note:** This only affects local development. npm consumers have a single `react-native` instance and are unaffected. +Only affects local development — npm consumers have a single `react-native` instance. diff --git a/.claude/rules/native-android.md b/.claude/rules/native-android.md index e0e642436..8b1bcc6c4 100644 --- a/.claude/rules/native-android.md +++ b/.claude/rules/native-android.md @@ -5,64 +5,51 @@ paths: # Native Android bridge rules -Scope: `android/` directory — `RNAppsFlyerModule.java`, `RNAppsFlyerPackage.java`, `RNAppsFlyerConstants.java`, `RNUtil.java`. +Scope: `android/` — `RNAppsFlyerModule.kt`, `RNAppsFlyerPackage.kt`, `RNAppsFlyerConstants.kt`, `RNUtil.java`. -## 1. Module structure +## Module structure -- `RNAppsFlyerModule extends ReactContextBaseJavaModule` — registered via `RNAppsFlyerPackage implements ReactPackage` -- Methods exposed with `@ReactMethod` annotation -- Method names match JS calls exactly (e.g., JS `initSdkWithCallBack` → Java `initSdkWithCallBack(ReadableMap, Callback, Callback)`) +- `RNAppsFlyerModule.kt` — TurboModule; extends `NativeAppsFlyerSpec`; `executeRpc(requestJson)` delegates into `AppsFlyerRpcHandler`, dispatching every RPC immediately, in submission order, on a single-thread executor — no listener-registration buffer. +- `RNAppsFlyerPackage.kt` — package registration +- `af-android-plugin-bridge` / `af-android-sdk` — real Maven dependencies (`android/build.gradle`), not vendored +- No `ReactContextBaseJavaModule` / `@ReactMethod` -## 2. CallbackGuard pattern (critical) +## Single entry point -Added in 6.17.8 to fix double-invocation crashes (#601). Wraps every `Callback` with: -- `AtomicBoolean` to ensure single invocation -- `WeakReference` to handle bridge destruction gracefully +One exported method: `executeRpc(requestJson: String): Promise`. Never add per-capability `@ReactMethod`s — add new capabilities in `AppsFlyerRpcHandler` instead. -```java -private static class CallbackGuard { - private final AtomicBoolean called = new AtomicBoolean(false); - private final WeakReference ref; - // invoke() checks-and-sets atomically -} -``` +## Threading -**Every new method that accepts a Callback must use CallbackGuard.** The React Native bridge crashes if a callback is invoked more than once — this is not optional. +Any RPC that can block natively (Android's `awaitResponse` model — up to 5–10s on `start`, `logEvent`, purchase validation) must be dispatched off the calling thread inside `RNAppsFlyerModule.kt`. Never call blocking RPC methods directly on the JS thread. -## 3. Constants export +## CallbackGuard — do not use in TurboModule -`getConstants()` exports `AFInAppEventType.*` constants to JS. These are available in JS as `RNAppsFlyer.ACHIEVEMENT_UNLOCKED`, etc. +`CallbackGuard` (`WeakReference`) fixed a double-invocation/GC bug specific to the old-architecture `Callback` type. TurboModule Promises are held strongly by the bridge, so that bug doesn't exist here — don't add `CallbackGuard` to `RNAppsFlyerModule.kt`. It's still present in `PCAppsFlyer` (legacy bridge) — leave it there. -## 4. Event emission +## Constants -Uses `reactContext.getJSModule(DeviceEventManagerModule.RCTDeviceEventEmitter.class).emit(eventName, data)`. Data is serialized to a JSON string before emission (matching iOS behavior). +`PLUGIN_VERSION` in `RNAppsFlyerConstants.kt` — keep in sync with the other 2 version locations (see `release-versioning.md`). -## 5. NativeEventEmitter stubs +`AFInAppEventType` constants live as a plain frozen object in `index.ts` — don't re-add them to `getConstants()`. -Lines ~1078-1085 in `RNAppsFlyerModule.java` have empty `addListener` and `removeListeners` method stubs annotated with `@ReactMethod`. These are required by RN's built-in `NativeEventEmitter` since RN 0.65. Do not remove them — their absence causes yellow-box warnings (#335). +## NativeEventEmitter stubs -## 6. Purchase Connector conditional compilation +`RNAppsFlyerModule.kt` must implement empty `addListener(eventName: String)` and `removeListeners(count: Double)` — required by `NativeEventEmitter`, absence causes warnings. -Gradle `sourceSets` conditionally includes `includeConnector` or `excludeConnector` directory based on the `appsflyer.enable_purchase_connector` gradle property. This toggles whether `PCAppsFlyer` Java classes are compiled. +## Event emission -## 7. Version constant +Emitted via `reactApplicationContext.emitDeviceEvent("RNAppsFlyer_rpcEvent", payload)` — a serialized JSON string, one shared event name for all event types. `@appsflyer-sdk/js-core-plugin` demuxes on `envelope.event` (see `bridge-patterns.md`). -`PLUGIN_VERSION` in `RNAppsFlyerConstants.java` — must be updated on every release, synchronized with the other 3 version locations. +## RNUtil -## 8. Namespace requirement (AGP 8+) +`RNUtil.java` handles `ReadableMap` ↔ JSON conversion. Still used where `ReadableMap` survives (e.g. `PCAppsFlyer`). -`build.gradle` must include `namespace` for Android Gradle Plugin 8.0+. This was added in plugin 6.15.1. Older versions cause `Namespace not specified` build failures (#583, #561). +## Build setup -## 9. Common Android build failures from issues +`android/build.gradle` pins `af-android-plugin-bridge:7.0.12` explicitly — `af-android-sdk-bom:7.0.1` doesn't carry a matching version. `namespace` is declared for AGP 8.0+. `minSdkVersion` defaults to 21 — verify `plugin_bridge`'s own `minSdkVersion` is ≤21 before release. -| Symptom | Root cause | Fix | -|---------|-----------|-----| -| `Namespace not specified` (#583, #561) | AGP 8+ requires namespace in build.gradle | Upgrade plugin to 6.15.1+ | -| `Multiple entries: android:allowBackup=REPLACE` (#627) | AndroidManifest merge conflict | Add `tools:replace` in app's main manifest | -| `ConcurrentModificationException` (#447) | Thread safety in native SDK | Upgrade native SDK | -| `IllegalAccessException on logEvent` (#464) | Reflection issue in native SDK | Upgrade native SDK | -| `null is not an object (RNAppsFlyer.logEvent)` (#333) | Autolinking not triggered | Run Gradle sync, clear Metro cache | +`AppsFlyerRpcHandler`'s constructor takes `contextProvider: () -> Context`, not `context: Context` — a named-arg call using `context =` fails to compile. `RNAppsFlyerModule.kt` passes `contextProvider = { reactApplicationContext.currentActivity ?: reactApplicationContext }`, invoked fresh on every call (not cached), so `init()` can backfill a missed `onActivityResumed` transition instead of stalling session-ready. -## 10. ReadableMap conversion +## Common Android build failures -`RNUtil.java` handles `ReadableMap` ↔ JSON conversion. When adding new methods that accept complex objects from JS, use `RNUtil` for conversion — do not write custom conversion logic. +Namespace/manifest-merge/thread-safety issues — see `known-issues-kb.md`'s "Android build failures" and "Runtime crashes" sections. diff --git a/.claude/rules/native-ios.md b/.claude/rules/native-ios.md index e41476ec2..56f297cc8 100644 --- a/.claude/rules/native-ios.md +++ b/.claude/rules/native-ios.md @@ -5,61 +5,49 @@ paths: # Native iOS bridge rules -Scope: `ios/` directory — `RNAppsFlyer.h`, `RNAppsFlyer.m`, `PCAppsFlyer.h`, `PCAppsFlyer.m`, `AppsFlyerAttribution.h/.m`. +Scope: `ios/` — `RNAppsFlyer.mm`, `RNAppsFlyer.h`, `RNAppsFlyerImpl.swift`, `AppsFlyerAttribution.swift`, `RNAppsFlyer-Bridging-Header.h`. `PCAppsFlyer.h/.m` (purchase connector) is legacy, out of scope. -## 1. Module structure +## Module structure -- `RNAppsFlyer` extends `RCTEventEmitter` (not `RCTBridgeModule` directly) — this enables `sendEventWithName:body:` -- Conforms to `AppsFlyerLibDelegate` and `AppsFlyerDeepLinkDelegate` -- Registered via `RCT_EXPORT_MODULE()` with no custom name +- `RNAppsFlyer.mm` — thin ObjC++ TurboModule shim, delegates to `RNAppsFlyerImpl.swift` +- `RNAppsFlyerImpl.swift` — RPC dispatch into `AppsFlyerRPCBridge`, event-channel wiring +- `AppsFlyerRPC` is a real CocoaPods dependency (podspec), not vendored +- No `RCTEventEmitter` — events go through the TurboModule's `NativeEventEmitter` channel -## 2. Method export naming +## Single entry point -| JS call | ObjC selector | -|---------|--------------| -| `initSdkWithCallBack(options, success, error)` | `initSdkWithCallBack:successCallback:errorCallback:` | -| `initSdkWithPromise(options)` | `initSdkWithPromise:initSdkWithPromiseWithResolver:rejecter:` | -| `logEvent(name, values, success, error)` | `logEvent:eventValues:successCallback:errorCallback:` | -| `getAppsFlyerUID(callback)` | `getAppsFlyerUID:` | +One exported method: `executeRpc(requestJson: String) -> Promise`. Never add per-capability `RCT_EXPORT_METHOD`s — add new capabilities in the native `AppsFlyerRPCBridge` handler instead. -Follow the existing naming convention when adding new methods. Promise variants use `RCT_EXPORT_METHOD` with `resolver:(RCTPromiseResolveBlock)` and `rejecter:(RCTPromiseRejectBlock)`. +## Threading -## 3. Threading +- Wrap every `AppsFlyerRPCBridge.shared.executeJson`/`setEventHandler` call in `Task { @MainActor in ... }` — the bridge is `@MainActor`-isolated; `executeRpc` runs off-main by default, so a synchronous call is a compile error. Don't use `MainActor.assumeIsolated` — it traps off-main. +- The bridge's own `RPCQueue` serializes RPC order internally regardless of caller thread, so the `Task` hop doesn't affect ordering. +- Event emissions to JS must go through the TurboModule event emitter, not `performSelectorOnMainThread`. -- Delegate callbacks use `performSelectorOnMainThread:withObject:waitUntilDone:NO` to dispatch to main thread before emitting JS events -- `logCrossPromotionAndOpenStore` uses `dispatch_async(dispatch_get_main_queue(), ...)` for UI operations -- All event emissions to JS must happen on the main thread +## Listener registration — no buffer -## 4. IDFA strict mode +`RNAppsFlyerImpl.swift` dispatches every RPC (including `init` and listener registration) immediately, in submission order — native is init-order-independent by design. Don't add a registration buffer without confirming an actual native regression first (see `known-issues-kb.md`). -`#ifndef AFSDK_NO_IDFA` guards ATT-related code. The podspec supports `$RNAppsFlyerStrictMode` which uses `AppsFlyerFramework/AppsFlyerFrameworkStrict` — this excludes IDFA access entirely. +## `AppsFlyerAttribution` bridge-ready gate -When adding ATT or IDFA-dependent code, always wrap in `#ifndef AFSDK_NO_IDFA`. +`AppsFlyerAttribution.swift` buffers AppDelegate-level `continueUserActivity`/`handleOpen(url:options:)` calls (cold-start Universal Link / custom-scheme open) until `RNAppsFlyerImpl` flips `AppsFlyerAttribution.shared.bridgeReady = true` — which happens once the `start` RPC (not `init`) resolves. Without this, the OS can call into AppDelegate before JS has run `init`/`start`, resolving against an unconfigured host or losing the click with nobody listening. `handleLaunchOptions` has no such dependency and passes straight through. -## 5. Version constant +App-side AppDelegates (and the Expo plugin's injected template) must call `AppsFlyerAttribution.shared`, never `AppsFlyerLib.shared()` directly, for `continueUserActivity`/`handleOpen`/`handleLaunchOptions`. -`kAppsFlyerPluginVersion` in `RNAppsFlyer.h` — must be updated on every release. This is separate from the podspec version and package.json version (see release-versioning.md). +`requestJson.method` arrives already resolved to the platform wire name (e.g. `"initialize"`, `"registerDeeplinkListener"`) — `@appsflyer-sdk/js-core-plugin` resolves it in JS before the call reaches native. Any method-name comparison in this file must match the resolved name, not the canonical JS name. -## 6. Podspec dependency +## IDFA / strict mode -`react-native-appsflyer.podspec` pins the native SDK version via `s.dependency 'AppsFlyerFramework'`. Header-not-found errors (#633, #602, #646) are almost always caused by: -- Stale pod cache (fix: `pod deintegrate && pod install --repo-update`) -- Podfile.lock pinning a different native SDK version than the podspec expects -- Strict mode missing headers (`AppsFlyerFrameworkStrict` has different headers) +`#ifndef AFSDK_NO_IDFA` guards ATT-related code. `$RNAppsFlyerStrictMode` pulls `AppsFlyerFrameworkStrict`, which excludes IDFA entirely — wrap new ATT-dependent code in `#ifndef AFSDK_NO_IDFA`. -## 7. Event names +## Version constant -`supportedEvents` returns a fixed array. Adding a new event type requires: -1. Add to the `supportedEvents` array in `RNAppsFlyer.m` -2. Add matching event name constant on Android -3. Add listener registration method in `index.js` -4. Add type in `index.d.ts` +`kAppsFlyerPluginVersion` in `RNAppsFlyer.h` — keep in sync with the other 2 version locations (see `release-versioning.md`). -## 8. Common iOS build failures from issues +## Podspec -| Symptom | Root cause | Fix | -|---------|-----------|-----| -| `react_native_appsflyer-Swift.h not found` (#646) | Mixed Swift/ObjC without bridging header | Check Xcode build settings for Swift bridging | -| `AppsFlyerConsent.h not found` (#633) | Native SDK version mismatch | Match plugin version to compatible native SDK | -| `Redefinition of SUCCESS` (#497, #541) | Enum collision with other libs | Update to plugin version where enum was namespaced | -| `unsupported Swift architecture` (#656) | Release build architecture mismatch | Check `EXCLUDED_ARCHS` build settings | +`s.dependency 'AppsFlyerRPC', '7.0.13'` (real CocoaPods coordinate; `Strict` variant used when `$RNAppsFlyerStrictMode` is set). No vendored framework files. + +## Common build issues + +Header-not-found / symbol-collision issues — see `known-issues-kb.md`'s "iOS build failures" section. diff --git a/.claude/rules/release-versioning.md b/.claude/rules/release-versioning.md index ccb6c8611..0cabd6f43 100644 --- a/.claude/rules/release-versioning.md +++ b/.claude/rules/release-versioning.md @@ -7,88 +7,81 @@ paths: # Release and versioning rules -Scope: version bumps, CHANGELOG.md, release branches, native SDK alignment. - -## 1. Version surface — 4 files must stay in sync +## Version surface — 3 literals must stay in sync | File | Field | Example | |------|-------|---------| -| `package.json` | `"version"` | `"6.17.9"` | -| `react-native-appsflyer.podspec` | `s.version` | `'6.17.9'` | -| `ios/RNAppsFlyer.h` | `kAppsFlyerPluginVersion` | `@"6.17.9"` | -| `android/…/RNAppsFlyerConstants.java` | `PLUGIN_VERSION` | `"6.17.9"` | +| `package.json` | `"version"` | `"7.0.2"` | +| `ios/RNAppsFlyer.h` | `kAppsFlyerPluginVersion` | `@"7.0.2"` | +| `android/…/RNAppsFlyerConstants.kt` | `PLUGIN_VERSION` | `"7.0.2"` | -Missing any one of these causes version mismatch bugs. Historical commits that were solely version constant syncs: `45a0cfeb`, `20c80b46`, `0b19d154`. +`react-native-appsflyer.podspec` reads `s.version = pkg["version"]` from `package.json` at pod-install time — it is **not** a separate literal to edit; doing so has no effect. -## 2. Version scheme +## Version scheme -Plugin version mirrors native SDK major.minor, with its own patch: -- `6.17.9` = plugin wrapping iOS SDK 6.17.8 + Android SDK 6.17.5 -- The plugin patch number is independent of native SDK patch numbers +Plugin version mirrors native SDK major.minor, with its own independent patch number (e.g. `7.0.2` wraps iOS SDK `7.0.13` / Android `af-android-sdk` `7.0.1`). -## 3. Semver rules +## Semver rules -| Change type | Bump | Examples | +| Change type | Bump | Trigger | |-------------|------|---------| -| Major | Never happened in 6.x era | Would require: removed public method, changed signature | -| Minor | New API additions | 6.9.1 (separated initSDK/startSDK), 6.13.0 (DMA support), 6.17.0 (Purchase Connector) | -| Patch | Native SDK updates, bug fixes, doc-only | 6.17.7 (SDK-only), 6.17.8 (bug fixes + new flags) | +| Major | — | Removed public method, changed signature | +| Minor | New API additions | New exported method/constant | +| Patch | Native SDK updates, bug fixes, doc-only | Everything else | -## 4. Deprecation pattern +## Deprecation pattern -```js -// In index.js — runtime warning +```ts console.warn('validateAndLogInAppPurchase is deprecated. Use AppsFlyerPurchaseConnector instead.'); -// In index.d.ts — type annotation /** @deprecated Use AppsFlyerPurchaseConnector instead */ export function validateAndLogInAppPurchase(...): void; ``` -Deprecated methods must continue to work at runtime. Add a backward-compat test in `__tests__/compatibility.test.js`. +Deprecated methods must keep working at runtime. Add a backward-compat test in `__tests__/compatibility.test.js`. -## 5. CHANGELOG format +## CHANGELOG format ```markdown -## 6.17.9 - Release date: *2025-01-15* +## 7.0.2 + Release date: *2026-08-25* - React Native >> Description of change - React Native >> Another change ``` -Rules: -- Entries use `React Native >>` prefix -- Dates use ISO format (YYYY-MM-DD) -- Newest version at top (reverse chronological) -- Breaking changes get a separate "Breaking changes" subsection with before/after +- Entries use the `React Native >>` prefix +- Dates in ISO format (YYYY-MM-DD) +- Newest version at top +- Breaking changes get their own "Breaking changes" subsection with before/after -## 6. Branch naming +## Branch naming | Type | Pattern | Example | |------|---------|---------| -| Feature | `dev/DELIVERY-{ticket}/description` | `dev/DELIVERY-115184/update-6.17.9` | -| Release | `releases/6.x.x/6.{minor}.x/6.{minor}.{patch}-rc{N}` | `releases/6.x.x/6.17.x/6.17.9-rc3` | +| Feature | `dev/DELIVERY-{ticket}/description` | `dev/DELIVERY-128515/latest-rpc-module-update` | +| Release | `releases/{major}.x.x/{major}.{minor}.x/{version}-rc{N}` | `releases/7.x.x/7.0.x/7.0.2-rc1` | | Hotfix | `{author}-patch-{N}` | `al-af-patch-1` | -## 7. Tag convention +## Tag convention -Pre-6.x: tags use `v` prefix (`v1.2.0` through `v5.4.40`). The 6.x series has no tags — releases tracked via branches and CHANGELOG. +All tags use the `v` prefix (`v6.18.0`, `v7.0.2`, ...). `release.yml` tags every production release (`v$VERSION`) and every RC (`v$VERSION-rcN`). -## 8. Native SDK dependency update +## Native SDK dependency update -When updating the native SDK version: 1. Update `react-native-appsflyer.podspec` dependency version 2. Update `android/build.gradle` dependency version -3. Test that all existing bridge methods still compile against new headers -4. Check CHANGELOG of native SDK for breaking changes that affect the bridge -5. If native SDK added new APIs, decide whether to bridge them (minor bump if yes) +3. Confirm existing bridge methods still compile against new headers +4. Check the native SDK's CHANGELOG for breaking changes affecting the bridge +5. If new native APIs were added, decide whether to bridge them (minor bump if yes) -## 9. Release checklist +## Release checklist -1. All 4 version constants updated and matching -2. CHANGELOG.md updated with new entry at top +1. All 3 version constants updated and matching +2. CHANGELOG.md updated with a new entry at top 3. `npm test` passes 4. `npx tsc --noEmit` passes 5. Manual test on iOS simulator and Android emulator 6. Demo app builds and runs on both platforms + +See `release.yml`/`promote-release.yml` for the actual RC → QA → production automation. diff --git a/.claude/rules/testing.md b/.claude/rules/testing.md index a744ea61f..0a9a3609e 100644 --- a/.claude/rules/testing.md +++ b/.claude/rules/testing.md @@ -6,73 +6,62 @@ paths: # Testing conventions -Scope: `__tests__/` directory, `jest.config.js`, test-related changes. +## Framework and config -## 1. Framework and config - -- Jest via `react-native` preset with `ts-jest` for TypeScript test support -- Config: `jest.config.js` -- Setup: `__tests__/setup.js` — mocks `NativeModules.RNAppsFlyer` (every native method is `jest.fn()`) and `NativeEventEmitter` +- Jest via `react-native` preset with `ts-jest` for TypeScript support +- Setup: `__tests__/setup.js` — mocks `src/NativeAppsFlyer` so every `executeRpc` call returns a configurable resolved Promise; mocks `NativeEventEmitter` via RN's official manual mock - Run: `npm test` (jest with coverage) -## 2. Test files +## Test files | File | Focus | |------|-------| -| `__tests__/index.test.js` | Core API surface + event emitters (~80 tests) | -| `__tests__/compatibility.test.js` | Backward compat for consent, StoreKit, callbacks (~15 tests) | -| `__tests__/linting.test.js` | ESLint validation of source files (~6 tests) | -| `__tests__/purchase-connector.test.ts` | PurchaseConnector models + interface (~40 tests) | +| `index.test.js` | Core API surface — each typed wrapper calls `executeRpc` with the correct method name and params | +| `rpc-contract.test.js` | Generic `executeRpc` round-trip: normalized success/error shapes, event-channel pass-through, listener-registration wiring, unsupported-method normalization | +| `threading.test.js` | `start()` settles with a distinguishable timeout failure instead of hanging | +| `compatibility.test.js` | Backward compat for consent, StoreKit, callbacks | +| `linting.test.js` | ESLint validation of source files | +| `purchase-connector.test.ts` | PurchaseConnector models + interface (legacy bridge, unchanged) | -## 3. Test pattern: mock-and-verify +## Mock-and-verify pattern -All tests follow the same pattern: -1. Call the JS API method -2. Assert the correct **native method** was called with expected arguments -3. For event emitters: emit an event, assert the handler received correct data +All JS tests mock `src/NativeAppsFlyer.executeRpc` and assert against the serialized request: ```js -// Example pattern -appsFlyer.logEvent('af_purchase', { af_revenue: 10 }, successCB, errorCB); -expect(RNAppsFlyer.logEvent).toHaveBeenCalledWith('af_purchase', { af_revenue: 10 }, successCB, errorCB); +NativeAppsFlyer.executeRpc.mockResolvedValue(JSON.stringify({ success: true, data: null })); +await AppsFlyer.setCustomerUserId('uid-123'); +const [requestJson] = NativeAppsFlyer.executeRpc.mock.calls[0]; +expect(JSON.parse(requestJson)).toEqual({ method: 'setCustomerUserId', params: { customerId: 'uid-123' } }); ``` -No integration tests or native-level tests exist. All native modules are fully mocked. +Never assert on `NativeModules.RNAppsFlyer` — unused in the TurboModule path. -## 4. Event listener tests +## Event listener tests -Test both paths: -- Happy path: native emits valid JSON string → handler receives parsed object -- Parse failure: native emits invalid JSON → handler receives `AFParseJSONException` object +Use `freshModule()` (`rpc-contract.test.js`) for a clean module instance — it resets modules and re-requires `index.ts` + `NativeAppsFlyer` + `NativeEventEmitter` fresh, since listener-registration state is module-level. Test the event channel by constructing a `NativeEventEmitter` from the fresh mock and calling `.emit('RNAppsFlyer_rpcEvent', envelopePayload)` directly. -## 5. Compatibility tests +## Compatibility tests -`compatibility.test.js` verifies deprecated APIs still work at runtime. When deprecating a method, add a test here proving the old call signature still routes correctly. +`compatibility.test.js` verifies the public API still works for known patterns. On a breaking change, update/remove the relevant compat test and add a migration-guide pointer. -## 6. Linting-as-tests +## Linting-as-tests -`linting.test.js` runs ESLint programmatically inside Jest. This is unusual but ensures lint rules are enforced in CI even without a separate lint step. +`linting.test.js` runs ESLint programmatically inside Jest — enforces lint in CI without a separate lint step. -## 7. Coverage gaps (known) +## Adding a new RPC method -These areas have **no test coverage** — adding tests here is high-value: -- Expo config plugins (`expo/withAppsFlyer.js`, `expo/withAppsFlyerIos.js`, `expo/withAppsFlyerAndroid.js`) -- Native-level unit tests (no XCTest, no Android JUnit) -- `logAdRevenue`, `logLocation`, `logCrossPromotionImpression`, `logCrossPromotionAndOpenStore` -- Edge cases in event listener cleanup (multiple listeners, unmount timing) +1. `index.test.js`: assert the JS wrapper calls `executeRpc` with the exact method name and param object +2. `rpc-contract.test.js` (if relevant): normalized error-handling or event-demux behavior +3. No native-level test needed for the wrapper itself — native handlers are tested at the native tier -## 8. What to test when adding a new method +## Do not mock internals -1. JS API calls correct native method name with correct arguments -2. Promise variant returns a Promise (not undefined) -3. Callback variant invokes the provided callbacks -4. Input validation (if any) rejects invalid types -5. Add backward-compat test if the method replaces a deprecated one +Only mock `src/NativeAppsFlyer` (via `setup.js`) and `NativeEventEmitter` (official RN manual mock). Dispatch logic lives in `@appsflyer-sdk/js-core-plugin` — don't reach in and mock its internals; test through `index.ts`'s public exports. -## 9. Do not mock internals +## Avoid tautological tests -Tests should only mock `NativeModules` (via `setup.js`). Do not mock internal JS functions within `index.js` — test through the public API surface. +Don't assert constants equal themselves — a test must be able to fail if the implementation breaks. -## 10. Avoid tautological tests +## Known coverage gaps -Some existing tests assert constants equal themselves (e.g., `expect('ironsource').toBe('ironsource')`). Do not add more of these — they test nothing. +Native-level iOS XCTest / Android JUnit for the RPC handlers, Expo config plugin tests, and live-device quickstart scenarios (killed-state deep link, full parity) are not yet covered. diff --git a/.claude/rules/typescript-types.md b/.claude/rules/typescript-types.md index 9e14e69a2..3512d2aae 100644 --- a/.claude/rules/typescript-types.md +++ b/.claude/rules/typescript-types.md @@ -1,56 +1,23 @@ --- paths: - - "index.d.ts" - - "**/*.d.ts" + - "index.ts" --- -# TypeScript type definitions +# TypeScript type conventions -Scope: `index.d.ts` and any `.d.ts` files in the repo. +Scope: the `PurchaseConnector`/`RNTransport` types hand-maintained in `index.ts` (see root `CLAUDE.md` — no separate `index.d.ts`/`index.js`). Core RPC method types are owned by `@appsflyer-sdk/js-core-plugin`, re-exported via `export *` — not maintained here. Drift history: `known-issues-kb.md` → "Types don't match runtime". -## 1. Hand-maintained, not generated +## Rules -`index.d.ts` is manually maintained — not generated from source. It declares the module via `declare module "react-native-appsflyer"`. This means types can (and do) drift from actual runtime behavior. +1. **No `any` for known shapes.** Use `Record` for genuinely dynamic data, not `any`. +2. **Platform-conditional types**: where iOS and Android return different shapes, document both in JSDoc (`@platform ios`/`@platform android`); use a union type if the difference is structural. +3. **Deprecation**: mark with `@deprecated` JSDoc, keep the signature working at runtime until removal (see `release-versioning.md` §4). +4. **New exports**: every named export needs a matching type — there's no separate step, they're the same declaration. -## 2. Chronic drift problem +## Validation -This is a recurring source of issues (#670, #575, #475, #218, #194): -- Types say one shape, native returns another -- `any` fallback types defeat TypeScript's purpose -- Deep link data shape differs between iOS and Android, but types assume a single shape - -**When changing any JS API or native return value, update `index.d.ts` in the same PR.** - -## 3. Type conventions - -```typescript -// Callback overload + Promise overload pattern -export function initSdk(options: InitSdkOptions, successC?: SuccessCB, errorC?: ErrorCB): Promise; - -// Event listener registration — returns cleanup function -export function onDeepLink(callback: (data: UnifiedDeepLinkData) => void): () => void; - -// Enum-like frozen objects -export const AFPurchaseType: { SUBSCRIPTION: string; ONE_TIME_PURCHASE: string }; -``` - -## 4. Rules for modifying types - -1. **Match runtime**: types must reflect what native actually returns, not what the docs say it should return. Test on both platforms before updating. -2. **No `any` for known shapes**: if the native return type is known, type it. Use `Record` for truly dynamic data, not `any`. -3. **Platform-conditional types**: where iOS and Android return different shapes, document both in JSDoc. Use union types if the difference is structural. -4. **Deprecation**: mark deprecated methods with `@deprecated` JSDoc tag. Keep the type signature for backward compatibility until removal. -5. **New exports**: every named export from `index.js` needs a matching type in `index.d.ts`. Missing types = broken TypeScript consumers. - -## 5. Stale header - -The file header says "Sync with v5.1.1" — this is misleading (last real sync was long ago). Do not rely on this header for version tracking. - -## 6. Validation approach - -After changing types: ```bash -npx tsc --noEmit # catches type errors in PurchaseConnector TS files +npx tsc --noEmit # type-checks index.ts + PurchaseConnector TS files ``` -For `index.d.ts` specifically, manual review against the JS implementation is required — `tsc` doesn't validate `.d.ts` against `.js`. +`tsc` doesn't validate that a type matches actual runtime output — cross-check against `specs/001-turbomodule-rpc-bridge/data-model.md` §Method Catalog and test on both platforms. diff --git a/.claude/settings.json b/.claude/settings.json index 2f44d358a..b0996f7da 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -1,4 +1,7 @@ { + "env": { + "CLAUDE_AUTOCOMPACT_PCT_OVERRIDE": "80" + }, "permissions": { "allow": [ "Read(*)", @@ -31,10 +34,6 @@ "Bash(pod deintegrate*)" ] }, - "env": { - "CLAUDE_AUTOCOMPACT_PCT_OVERRIDE": "80" - }, - "plansDirectory": "./docs/plans", "hooks": { "PostToolUse": [ { @@ -48,5 +47,6 @@ ] } ] - } + }, + "plansDirectory": "./docs/plans" } diff --git a/.claude/skills/doctor-explain/SKILL.md b/.claude/skills/doctor-explain/SKILL.md new file mode 100644 index 000000000..c8608ef07 --- /dev/null +++ b/.claude/skills/doctor-explain/SKILL.md @@ -0,0 +1,75 @@ +--- +name: doctor-explain +description: Explain React Doctor rules and configure which ones run via doctor.config.* (or package.json#reactDoctor). Use when the user types `/doctor-explain` or `/doctor-config`, asks why a rule fired, disagrees with a rule, wants to disable/enable a rule, silence a category or tag, tune CI/PR noise, or asks "what does this rule mean". Covers the `react-doctor rules` CLI (list, explain, set, enable, disable, category, ignore-tag) and how config layers combine: ignore.tags disables matching rules before linting, rules over categories sets severity, surfaces controls visibility only. +version: "1.0.0" +--- + +# Doctor Explain + +Explains React Doctor rules and edits `doctor.config.*` safely. Use this when a user wants to understand a rule or change which rules run — not for fixing diagnostics (that is the `react-doctor` skill / `/doctor`). + +Triggers: `/doctor-explain`, `/doctor-config`, "why did this rule fire", "I disagree with this rule", "turn this rule off", "stop flagging X", "too noisy", "disable design rules". + +## Workflow + +1. Identify the rule key from the diagnostic (e.g. `react-doctor/no-array-index-as-key`). +2. Explain it before changing anything: + +```bash +npx react-doctor@latest rules explain react-doctor/no-array-index-as-key +``` + +3. Pick the narrowest control that matches the user's intent (see decision guide). +4. Apply it with a `rules` subcommand (edits your `doctor.config.*` or `package.json#reactDoctor` in place, preserving other fields and formatting). +5. Validate the change did what they wanted: + +```bash +npx react-doctor@latest --verbose --diff +``` + +## Commands + +```bash +npx react-doctor@latest rules list # every rule + its effective severity +npx react-doctor@latest rules list --configured # only what your config changed +npx react-doctor@latest rules list --category Performance # filter by category +npx react-doctor@latest rules explain # why it matters + how to configure +npx react-doctor@latest rules disable # rule never runs +npx react-doctor@latest rules enable # turn back on at its recommended severity +npx react-doctor@latest rules set warn # off | warn | error +npx react-doctor@latest rules category "React Native" off # whole category +npx react-doctor@latest rules ignore-tag design # skip a rule family (design, test-noise, …) +npx react-doctor@latest rules unignore-tag design +``` + +Rule references accept the full key (`react-doctor/no-danger`), the bare id (`no-danger`), or a legacy key (`react/no-danger`). + +## Decision guide + +Match the control to the intent — prefer the narrowest one: + +- **User disagrees with one rule / it's a false positive for them** → `rules disable ` (sets `rules. = "off"`; the rule stops running everywhere). This is the default for "I don't want this rule". +- **Rule is fine but wrong severity** → `rules set warn` or `rules set error`. +- **A disabled-by-default rule they want on** → `rules enable `. +- **A whole area is unwanted** (e.g. all React Native rules) → `rules category "" off`. +- **A behavioral family is noisy** (`design`, `test-noise`, `migration-hint`) → `rules ignore-tag `. +- **Keep it locally but hide from PR comment / score / CI gate only** → do NOT disable. Edit `surfaces` in your config (`surfaces.prComment.excludeRules`, `surfaces.score.excludeTags`, `surfaces.ciFailure.excludeCategories`). The rule still shows in local `cli` output. + +How the layers combine: `ignore.tags` disables every rule carrying that tag **before** linting, so a tagged rule stays off even if `rules`/`categories` set it to `warn`/`error` (a rule-level override cannot re-enable a tag-ignored rule). For rules that aren't tag-disabled, `rules` overrides `categories` overrides the rule's default. `surfaces` is visibility-only and never changes whether a rule runs. + +## Config shape + +Config lives in `doctor.config.ts` (or `.js`/`.mjs`/`.cjs`/`.json`/`.jsonc`), or the `reactDoctor` key in `package.json`. The `rules` commands edit whichever exists — TS/JS edits preserve formatting (via magicast) — and create `doctor.config.json` when none does, stamping `$schema`: + +```ts +// doctor.config.ts +export default { + rules: { "react-doctor/no-array-index-as-key": "off" }, + categories: { "React Native": "warn" }, + ignore: { tags: ["design"] }, +}; +``` + +## Educating the user + +When explaining a rule, lead with the "Why it matters" guidance from `rules explain` and, when they want depth, the per-rule recipe at `https://www.react.doctor/prompts/rules//.md`. Only after they understand it should you offer to disable it — many "bad" rules are catching real issues. diff --git a/.claude/skills/react-doctor/SKILL.md b/.claude/skills/react-doctor/SKILL.md new file mode 100644 index 000000000..3afcd67aa --- /dev/null +++ b/.claude/skills/react-doctor/SKILL.md @@ -0,0 +1,50 @@ +--- +name: react-doctor +description: Use when finishing a feature, fixing a bug, before committing React code, or when the user types `/doctor`, asks to scan, triage, or clean up React diagnostics. Covers lint, accessibility, bundle size, architecture. Includes a regression check and a full local-triage workflow that fetches the canonical playbook. +version: "1.1.0" +--- + +# React Doctor + +Scans React codebases for security, performance, correctness, and architecture issues. Outputs a 0–100 health score. + +## After making React code changes: + +Run `npx react-doctor@latest --verbose --diff` and check the score did not regress. + +If the score dropped, fix the regressions before committing. + +## For general cleanup or code improvement: + +Run `npx react-doctor@latest --verbose` (without `--diff`) to scan the full codebase. Fix issues by severity — errors first, then warnings. + +## /doctor — full local triage workflow + +When the user types `/doctor`, says "run react doctor", or asks for a full triage / cleanup pass (not just a regression check), fetch the canonical local-triage playbook and follow every step in it: + +```bash +curl --fail --silent --show-error \ + --header 'Cache-Control: no-cache' \ + https://www.react.doctor/prompts/react-doctor-agent.md +``` + +The playbook is the single source of truth — a scan → filter → triage → fix → validate loop that edits the working tree directly (never commits, never opens PRs). Updating the prompt at its source updates every agent on its next fetch — no skill reinstall needed. + +Pair it with the matching per-rule prompts at `https://www.react.doctor/prompts/rules//.md` (fetched on demand inside the playbook) so each fix uses the canonical, reviewer-tested recipe. + +## Configuring or explaining rules + +When the user wants to understand a rule, disagrees with one, or wants to disable / tune which rules run (not fix code), use the `doctor-explain` skill (alias `/doctor-config`). Start with `npx react-doctor@latest rules explain `, then apply the narrowest control via `npx react-doctor@latest rules disable|set|category|ignore-tag …`, which edits your `doctor.config.*` (or `package.json#reactDoctor`). + +## Command + +```bash +npx react-doctor@latest --verbose --diff +``` + +| Flag | Purpose | +| ----------- | --------------------------------------------- | +| `.` | Scan current directory | +| `--verbose` | Show affected files and line numbers per rule | +| `--diff` | Only scan changed files vs base branch | +| `--score` | Output only the numeric score | diff --git a/.claude/skills/rn-architect-guardrails/SKILL.md b/.claude/skills/rn-architect-guardrails/SKILL.md new file mode 100644 index 000000000..7f70df3d5 --- /dev/null +++ b/.claude/skills/rn-architect-guardrails/SKILL.md @@ -0,0 +1,66 @@ +--- +name: rn-architect-guardrails +description: Reviews and refactors React Native / Expo demo code (App.js, components) to eliminate common AI-generated anti-patterns — useEffect state-syncing, unjustified useMemo/useCallback, business logic trapped inside component bodies, and missing FlatList perf props. Use when writing or reviewing React Native components, especially in demos/appsflyer-expo-app, or when the user asks to "review this component", "clean up this hook", or "audit for React anti-patterns". +--- + +# React Native Architect Guardrails + +Enforce these 4 rules on any React Native component you write or review. Fix violations in place; don't just flag them. + +## 1. Derived state over `useEffect` + +Never use `useEffect` + a state setter to sync/filter/format/map data that's computable during render. Delete the effect and the secondary `useState`; compute inline. + +```js +// BAD +const [data, setData] = useState([]); +const [filtered, setFiltered] = useState([]); +useEffect(() => { setFiltered(data.filter(i => i.active)) }, [data]); + +// GOOD +const filtered = data.filter(i => i.active); +``` + +Same rule for state that should reset when a prop changes: don't `useEffect` to detect the prop change and call a setter — give the component a `key={propValue}` so React remounts it fresh instead. + +## 2. Justified `useMemo`/`useCallback` only + +Ban memoization for standard handlers or cheap transforms. Only keep it when either: +- the value/function is passed to a child wrapped in `React.memo()`, or +- it's an expensive computation (heavy regex, array ops over 1000+ items). + +```js +// BAD +const onPress = useCallback(() => navigate('Home'), []); +// GOOD +const onPress = () => navigate('Home'); +``` + +Exception: `FlatList`'s `renderItem` counts as the `React.memo()` case — FlatList recycles item renderers internally, so an inline arrow function defined in JSX defeats that. Wrap it in `useCallback` (or define it outside the component if it doesn't close over props/state). + +## 3. Pure function extraction + +Any function inside a component that doesn't read/write state, props, or hooks belongs outside the component (bottom of file or a util module) as a plain, testable function. + +```js +// BAD +function MyComponent({ text }) { + const formatText = (str) => str.toUpperCase(); + return {formatText(text)}; +} +// GOOD +const formatText = (str) => str.toUpperCase(); +function MyComponent({ text }) { + return {formatText(text)}; +} +``` + +## 4. `FlatList` perf props + +For any `FlatList` with a large or frequently-updated `data` array: give it a stable `keyExtractor` (already required), a `useCallback`-wrapped `renderItem` (see rule 2 exception), and, if every row is the same height/width, `getItemLayout={(data, index) => ({length, offset: length * index, index})}` — it skips FlatList's async layout measurement pass entirely. + +## Workflow + +1. Scan the target file(s) for `useEffect`, `useMemo`, `useCallback`, non-hook helper functions declared inside components, and `FlatList` usage. +2. For each hit, apply the matching rule above and rewrite in place. +3. Report each fix as one line: `file:line — rule violated → what changed`. diff --git a/.claude/skills/speckit-analyze/SKILL.md b/.claude/skills/speckit-analyze/SKILL.md new file mode 100644 index 000000000..83cd91be4 --- /dev/null +++ b/.claude/skills/speckit-analyze/SKILL.md @@ -0,0 +1,262 @@ +--- +name: "speckit-analyze" +description: "Perform a non-destructive cross-artifact consistency and quality analysis across spec.md, plan.md, and tasks.md after task generation." +argument-hint: "Optional focus areas for analysis" +compatibility: "Requires spec-kit project structure with .specify/ directory" +metadata: + author: "github-spec-kit" + source: "templates/commands/analyze.md" +user-invocable: true +disable-model-invocation: false +--- + + +## User Input + +```text +$ARGUMENTS +``` + +You **MUST** consider the user input before proceeding (if not empty). + +## Pre-Execution Checks + +**Check for extension hooks (before analysis)**: +- Check if `.specify/extensions.yml` exists in the project root. +- If it exists, read it and look for entries under the `hooks.before_analyze` key +- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally +- Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. +- For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: + - If the hook has no `condition` field, or it is null/empty, treat the hook as executable + - If the hook defines a non-empty `condition`, skip the hook and leave condition evaluation to the HookExecutor implementation +- When constructing slash commands from hook command names, replace dots (`.`) with hyphens (`-`). For example, `speckit.git.commit` → `/speckit-git-commit`. +- For each executable hook, output the following based on its `optional` flag: + - **Optional hook** (`optional: true`): + ``` + ## Extension Hooks + + **Optional Pre-Hook**: {extension} + Command: `/{command}` + Description: {description} + + Prompt: {prompt} + To execute: `/{command}` + ``` + - **Mandatory hook** (`optional: false`): + ``` + ## Extension Hooks + + **Automatic Pre-Hook**: {extension} + Executing: `/{command}` + EXECUTE_COMMAND: {command} + + Wait for the result of the hook command before proceeding to the Goal. + ``` + After emitting the block above you MUST actually invoke the hook and wait for it to finish before continuing. Run it the same way you would run the command yourself in this agent/session (the invocation may differ from the literal `{command}` id shown above, e.g. a skills-mode agent runs it as `/skill:speckit-...` or `$speckit-...`). Emitting the block alone does not run the hook. +- If no hooks are registered or `.specify/extensions.yml` does not exist, skip silently + +## Goal + +Identify inconsistencies, duplications, ambiguities, and underspecified items across the three core artifacts (`spec.md`, `plan.md`, `tasks.md`) before implementation. This command MUST run only after `/speckit-tasks` has successfully produced a complete `tasks.md`. + +## Operating Constraints + +**STRICTLY READ-ONLY**: Do **not** modify any files. Output a structured analysis report. Offer an optional remediation plan (user must explicitly approve before any follow-up editing commands would be invoked manually). + +**Constitution Authority**: The project constitution (`.specify/memory/constitution.md`) is **non-negotiable** within this analysis scope. Constitution conflicts are automatically CRITICAL and require adjustment of the spec, plan, or tasks—not dilution, reinterpretation, or silent ignoring of the principle. If a principle itself needs to change, that must occur in a separate, explicit constitution update outside `/speckit-analyze`. + +## Execution Steps + +### 1. Initialize Analysis Context + +Run `.specify/scripts/bash/check-prerequisites.sh --json --require-tasks --include-tasks` once from repo root and parse JSON for FEATURE_DIR and AVAILABLE_DOCS. Derive absolute paths: + +- SPEC = FEATURE_DIR/spec.md +- PLAN = FEATURE_DIR/plan.md +- TASKS = FEATURE_DIR/tasks.md + +Abort with an error message if any required file is missing (instruct the user to run missing prerequisite command). +For single quotes in args like "I'm Groot", use escape syntax: e.g 'I'\''m Groot' (or double-quote if possible: "I'm Groot"). + +### 2. Load Artifacts (Progressive Disclosure) + +Load only the minimal necessary context from each artifact: + +**From spec.md:** + +- Overview/Context +- Functional Requirements +- Success Criteria (measurable outcomes — e.g., performance, security, availability, user success, business impact) +- User Stories +- Edge Cases (if present) + +**From plan.md:** + +- Architecture/stack choices +- Data Model references +- Phases +- Technical constraints + +**From tasks.md:** + +- Task IDs +- Descriptions +- Phase grouping +- Parallel markers [P] +- Referenced file paths + +**From constitution:** + +- Load `.specify/memory/constitution.md` for principle validation + +### 3. Build Semantic Models + +Create internal representations (do not include raw artifacts in output): + +- **Requirements inventory**: For each Functional Requirement (FR-###) and Success Criterion (SC-###), record a stable key. Use the explicit FR-/SC- identifier as the primary key when present, and optionally also derive an imperative-phrase slug for readability (e.g., "User can upload file" → `user-can-upload-file`). Include only Success Criteria items that require buildable work (e.g., load-testing infrastructure, security audit tooling), and exclude post-launch outcome metrics and business KPIs (e.g., "Reduce support tickets by 50%"). +- **User story/action inventory**: Discrete user actions with acceptance criteria +- **Task coverage mapping**: Map each task to one or more requirements or stories (inference by keyword / explicit reference patterns like IDs or key phrases) +- **Constitution rule set**: Extract principle names and MUST/SHOULD normative statements + +### 4. Detection Passes (Token-Efficient Analysis) + +Focus on high-signal findings. Limit to 50 findings total; aggregate remainder in overflow summary. + +#### A. Duplication Detection + +- Identify near-duplicate requirements +- Mark lower-quality phrasing for consolidation + +#### B. Ambiguity Detection + +- Flag vague adjectives (fast, scalable, secure, intuitive, robust) lacking measurable criteria +- Flag unresolved placeholders (TODO, TKTK, ???, ``, etc.) + +#### C. Underspecification + +- Requirements with verbs but missing object or measurable outcome +- User stories missing acceptance criteria alignment +- Tasks referencing files or components not defined in spec/plan + +#### D. Constitution Alignment + +- Any requirement or plan element conflicting with a MUST principle +- Missing mandated sections or quality gates from constitution + +#### E. Coverage Gaps + +- Requirements with zero associated tasks +- Tasks with no mapped requirement/story +- Success Criteria requiring buildable work (performance, security, availability) not reflected in tasks + +#### F. Inconsistency + +- Terminology drift (same concept named differently across files) +- Data entities referenced in plan but absent in spec (or vice versa) +- Task ordering contradictions (e.g., integration tasks before foundational setup tasks without dependency note) +- Conflicting requirements (e.g., one requires Next.js while other specifies Vue) + +### 5. Severity Assignment + +Use this heuristic to prioritize findings: + +- **CRITICAL**: Violates constitution MUST, missing core spec artifact, or requirement with zero coverage that blocks baseline functionality +- **HIGH**: Duplicate or conflicting requirement, ambiguous security/performance attribute, untestable acceptance criterion +- **MEDIUM**: Terminology drift, missing non-functional task coverage, underspecified edge case +- **LOW**: Style/wording improvements, minor redundancy not affecting execution order + +### 6. Produce Compact Analysis Report + +Output a Markdown report (no file writes) with the following structure: + +## Specification Analysis Report + +| ID | Category | Severity | Location(s) | Summary | Recommendation | +|----|----------|----------|-------------|---------|----------------| +| A1 | Duplication | HIGH | spec.md:L120-134 | Two similar requirements ... | Merge phrasing; keep clearer version | + +(Add one row per finding; generate stable IDs prefixed by category initial.) + +**Coverage Summary Table:** + +| Requirement Key | Has Task? | Task IDs | Notes | +|-----------------|-----------|----------|-------| + +**Constitution Alignment Issues:** (if any) + +**Unmapped Tasks:** (if any) + +**Metrics:** + +- Total Requirements +- Total Tasks +- Coverage % (requirements with >=1 task) +- Ambiguity Count +- Duplication Count +- Critical Issues Count + +### 7. Provide Next Actions + +At end of report, output a concise Next Actions block: + +- If CRITICAL issues exist: Recommend resolving before `/speckit-implement` +- If only LOW/MEDIUM: User may proceed, but provide improvement suggestions +- Provide explicit command suggestions: e.g., "Run /speckit-specify with refinement", "Run /speckit-plan to adjust architecture", "Manually edit tasks.md to add coverage for 'performance-metrics'" + +### 8. Offer Remediation + +Ask the user: "Would you like me to suggest concrete remediation edits for the top N issues?" (Do NOT apply them automatically.) + +### 9. Check for extension hooks + +After reporting, check if `.specify/extensions.yml` exists in the project root. +- If it exists, read it and look for entries under the `hooks.after_analyze` key +- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally +- Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. +- For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: + - If the hook has no `condition` field, or it is null/empty, treat the hook as executable + - If the hook defines a non-empty `condition`, skip the hook and leave condition evaluation to the HookExecutor implementation +- When constructing slash commands from hook command names, replace dots (`.`) with hyphens (`-`). For example, `speckit.git.commit` → `/speckit-git-commit`. +- For each executable hook, output the following based on its `optional` flag: + - **Optional hook** (`optional: true`): + ``` + ## Extension Hooks + + **Optional Hook**: {extension} + Command: `/{command}` + Description: {description} + + Prompt: {prompt} + To execute: `/{command}` + ``` + - **Mandatory hook** (`optional: false`): + ``` + ## Extension Hooks + + **Automatic Hook**: {extension} + Executing: `/{command}` + EXECUTE_COMMAND: {command} + ``` + After emitting the block above you MUST actually invoke the hook and wait for it to finish before continuing. Run it the same way you would run the command yourself in this agent/session (the invocation may differ from the literal `{command}` id shown above, e.g. a skills-mode agent runs it as `/skill:speckit-...` or `$speckit-...`). Emitting the block alone does not run the hook. +- If no hooks are registered or `.specify/extensions.yml` does not exist, skip silently + +## Operating Principles + +### Context Efficiency + +- **Minimal high-signal tokens**: Focus on actionable findings, not exhaustive documentation +- **Progressive disclosure**: Load artifacts incrementally; don't dump all content into analysis +- **Token-efficient output**: Limit findings table to 50 rows; summarize overflow +- **Deterministic results**: Rerunning without changes should produce consistent IDs and counts + +### Analysis Guidelines + +- **NEVER modify files** (this is read-only analysis) +- **NEVER hallucinate missing sections** (if absent, report them accurately) +- **Prioritize constitution violations** (these are always CRITICAL) +- **Use examples over exhaustive rules** (cite specific instances, not generic patterns) +- **Report zero issues gracefully** (emit success report with coverage statistics) + +## Context + +$ARGUMENTS diff --git a/.claude/skills/speckit-checklist/SKILL.md b/.claude/skills/speckit-checklist/SKILL.md new file mode 100644 index 000000000..927ef4aa8 --- /dev/null +++ b/.claude/skills/speckit-checklist/SKILL.md @@ -0,0 +1,376 @@ +--- +name: "speckit-checklist" +description: "Generate a custom checklist for the current feature based on user requirements." +argument-hint: "Domain or focus area for the checklist" +compatibility: "Requires spec-kit project structure with .specify/ directory" +metadata: + author: "github-spec-kit" + source: "templates/commands/checklist.md" +user-invocable: true +disable-model-invocation: false +--- + + +## Checklist Purpose: "Unit Tests for English" + +**CRITICAL CONCEPT**: Checklists are **UNIT TESTS FOR REQUIREMENTS WRITING** - they validate the quality, clarity, and completeness of requirements in a given domain. + +**NOT for verification/testing**: + +- ❌ NOT "Verify the button clicks correctly" +- ❌ NOT "Test error handling works" +- ❌ NOT "Confirm the API returns 200" +- ❌ NOT checking if code/implementation matches the spec + +**FOR requirements quality validation**: + +- ✅ "Are visual hierarchy requirements defined for all card types?" (completeness) +- ✅ "Is 'prominent display' quantified with specific sizing/positioning?" (clarity) +- ✅ "Are hover state requirements consistent across all interactive elements?" (consistency) +- ✅ "Are accessibility requirements defined for keyboard navigation?" (coverage) +- ✅ "Does the spec define what happens when logo image fails to load?" (edge cases) + +**Metaphor**: If your spec is code written in English, the checklist is its unit test suite. You're testing whether the requirements are well-written, complete, unambiguous, and ready for implementation - NOT whether the implementation works. + +## User Input + +```text +$ARGUMENTS +``` + +You **MUST** consider the user input before proceeding (if not empty). + +## Pre-Execution Checks + +**Check for extension hooks (before checklist generation)**: +- Check if `.specify/extensions.yml` exists in the project root. +- If it exists, read it and look for entries under the `hooks.before_checklist` key +- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally +- Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. +- For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: + - If the hook has no `condition` field, or it is null/empty, treat the hook as executable + - If the hook defines a non-empty `condition`, skip the hook and leave condition evaluation to the HookExecutor implementation +- When constructing slash commands from hook command names, replace dots (`.`) with hyphens (`-`). For example, `speckit.git.commit` → `/speckit-git-commit`. +- For each executable hook, output the following based on its `optional` flag: + - **Optional hook** (`optional: true`): + ``` + ## Extension Hooks + + **Optional Pre-Hook**: {extension} + Command: `/{command}` + Description: {description} + + Prompt: {prompt} + To execute: `/{command}` + ``` + - **Mandatory hook** (`optional: false`): + ``` + ## Extension Hooks + + **Automatic Pre-Hook**: {extension} + Executing: `/{command}` + EXECUTE_COMMAND: {command} + + Wait for the result of the hook command before proceeding to the Execution Steps. + ``` + After emitting the block above you MUST actually invoke the hook and wait for it to finish before continuing. Run it the same way you would run the command yourself in this agent/session (the invocation may differ from the literal `{command}` id shown above, e.g. a skills-mode agent runs it as `/skill:speckit-...` or `$speckit-...`). Emitting the block alone does not run the hook. +- If no hooks are registered or `.specify/extensions.yml` does not exist, skip silently + +## Execution Steps + +1. **Setup**: Run `.specify/scripts/bash/check-prerequisites.sh --json` from repo root and parse JSON for FEATURE_DIR and AVAILABLE_DOCS list. + - All file paths must be absolute. + - For single quotes in args like "I'm Groot", use escape syntax: e.g 'I'\''m Groot' (or double-quote if possible: "I'm Groot"). + +2. **IF EXISTS**: Load `.specify/memory/constitution.md` for project principles and governance constraints. + +3. **Clarify intent (dynamic)**: Derive up to THREE initial contextual clarifying questions (no pre-baked catalog). They MUST: + - Be generated from the user's phrasing + extracted signals from spec/plan/tasks + - Only ask about information that materially changes checklist content + - Be skipped individually if already unambiguous in `$ARGUMENTS` + - Prefer precision over breadth + + Generation algorithm: + 1. Extract signals: feature domain keywords (e.g., auth, latency, UX, API), risk indicators ("critical", "must", "compliance"), stakeholder hints ("QA", "review", "security team"), and explicit deliverables ("a11y", "rollback", "contracts"). + 2. Cluster signals into candidate focus areas (max 4) ranked by relevance. + 3. Identify probable audience & timing (author, reviewer, QA, release) if not explicit. + 4. Detect missing dimensions: scope breadth, depth/rigor, risk emphasis, exclusion boundaries, measurable acceptance criteria. + 5. Formulate questions chosen from these archetypes: + - Scope refinement (e.g., "Should this include integration touchpoints with X and Y or stay limited to local module correctness?") + - Risk prioritization (e.g., "Which of these potential risk areas should receive mandatory gating checks?") + - Depth calibration (e.g., "Is this a lightweight pre-commit sanity list or a formal release gate?") + - Audience framing (e.g., "Will this be used by the author only or peers during PR review?") + - Boundary exclusion (e.g., "Should we explicitly exclude performance tuning items this round?") + - Scenario class gap (e.g., "No recovery flows detected—are rollback / partial failure paths in scope?") + + Question formatting rules: + - If presenting options, generate a compact table with columns: Option | Candidate | Why It Matters + - Limit to A–E options maximum; omit table if a free-form answer is clearer + - Never ask the user to restate what they already said + - Avoid speculative categories (no hallucination). If uncertain, ask explicitly: "Confirm whether X belongs in scope." + + Defaults when interaction impossible: + - Depth: Standard + - Audience: Reviewer (PR) if code-related; Author otherwise + - Focus: Top 2 relevance clusters + + Output the questions (label Q1/Q2/Q3). After answers: if ≥2 scenario classes (Alternate / Exception / Recovery / Non-Functional domain) remain unclear, you MAY ask up to TWO more targeted follow‑ups (Q4/Q5) with a one-line justification each (e.g., "Unresolved recovery path risk"). Do not exceed five total questions. Skip escalation if user explicitly declines more. + +4. **Understand user request**: Combine `$ARGUMENTS` + clarifying answers: + - Derive checklist theme (e.g., security, review, deploy, ux) + - Consolidate explicit must-have items mentioned by user + - Map focus selections to category scaffolding + - Infer any missing context from spec/plan/tasks (do NOT hallucinate) + +5. **Load feature context**: Read from FEATURE_DIR: + - spec.md: Feature requirements and scope + - plan.md (if exists): Technical details, dependencies + - tasks.md (if exists): Implementation tasks + + **Context Loading Strategy**: + - Load only necessary portions relevant to active focus areas (avoid full-file dumping) + - Prefer summarizing long sections into concise scenario/requirement bullets + - Use progressive disclosure: add follow-on retrieval only if gaps detected + - If source docs are large, generate interim summary items instead of embedding raw text + +6. **Generate checklist** - Create "Unit Tests for Requirements": + - Create `FEATURE_DIR/checklists/` directory if it doesn't exist + - Generate unique checklist filename: + - Use short, descriptive name based on domain (e.g., `ux.md`, `api.md`, `security.md`) + - Format: `[domain].md` + - File handling behavior: + - If file does NOT exist: Create new file and number items starting from CHK001 + - If file exists: Append new items to existing file, continuing from the last CHK ID (e.g., if last item is CHK015, start new items at CHK016) + - Never delete or replace existing checklist content - always preserve and append + + **CORE PRINCIPLE - Test the Requirements, Not the Implementation**: + Every checklist item MUST evaluate the REQUIREMENTS THEMSELVES for: + - **Completeness**: Are all necessary requirements present? + - **Clarity**: Are requirements unambiguous and specific? + - **Consistency**: Do requirements align with each other? + - **Measurability**: Can requirements be objectively verified? + - **Coverage**: Are all scenarios/edge cases addressed? + + **Category Structure** - Group items by requirement quality dimensions: + - **Requirement Completeness** (Are all necessary requirements documented?) + - **Requirement Clarity** (Are requirements specific and unambiguous?) + - **Requirement Consistency** (Do requirements align without conflicts?) + - **Acceptance Criteria Quality** (Are success criteria measurable?) + - **Scenario Coverage** (Are all flows/cases addressed?) + - **Edge Case Coverage** (Are boundary conditions defined?) + - **Non-Functional Requirements** (Performance, Security, Accessibility, etc. - are they specified?) + - **Dependencies & Assumptions** (Are they documented and validated?) + - **Ambiguities & Conflicts** (What needs clarification?) + + **HOW TO WRITE CHECKLIST ITEMS - "Unit Tests for English"**: + + ❌ **WRONG** (Testing implementation): + - "Verify landing page displays 3 episode cards" + - "Test hover states work on desktop" + - "Confirm logo click navigates home" + + ✅ **CORRECT** (Testing requirements quality): + - "Are the exact number and layout of featured episodes specified?" [Completeness] + - "Is 'prominent display' quantified with specific sizing/positioning?" [Clarity] + - "Are hover state requirements consistent across all interactive elements?" [Consistency] + - "Are keyboard navigation requirements defined for all interactive UI?" [Coverage] + - "Is the fallback behavior specified when logo image fails to load?" [Edge Cases] + - "Are loading states defined for asynchronous episode data?" [Completeness] + - "Does the spec define visual hierarchy for competing UI elements?" [Clarity] + + **ITEM STRUCTURE**: + Each item should follow this pattern: + - Question format asking about requirement quality + - Focus on what's WRITTEN (or not written) in the spec/plan + - Include quality dimension in brackets [Completeness/Clarity/Consistency/etc.] + - Reference spec section `[Spec §X.Y]` when checking existing requirements + - Use `[Gap]` marker when checking for missing requirements + + **EXAMPLES BY QUALITY DIMENSION**: + + Completeness: + - "Are error handling requirements defined for all API failure modes? [Gap]" + - "Are accessibility requirements specified for all interactive elements? [Completeness]" + - "Are mobile breakpoint requirements defined for responsive layouts? [Gap]" + + Clarity: + - "Is 'fast loading' quantified with specific timing thresholds? [Clarity, Spec §NFR-2]" + - "Are 'related episodes' selection criteria explicitly defined? [Clarity, Spec §FR-5]" + - "Is 'prominent' defined with measurable visual properties? [Ambiguity, Spec §FR-4]" + + Consistency: + - "Do navigation requirements align across all pages? [Consistency, Spec §FR-10]" + - "Are card component requirements consistent between landing and detail pages? [Consistency]" + + Coverage: + - "Are requirements defined for zero-state scenarios (no episodes)? [Coverage, Edge Case]" + - "Are concurrent user interaction scenarios addressed? [Coverage, Gap]" + - "Are requirements specified for partial data loading failures? [Coverage, Exception Flow]" + + Measurability: + - "Are visual hierarchy requirements measurable/testable? [Acceptance Criteria, Spec §FR-1]" + - "Can 'balanced visual weight' be objectively verified? [Measurability, Spec §FR-2]" + + **Scenario Classification & Coverage** (Requirements Quality Focus): + - Check if requirements exist for: Primary, Alternate, Exception/Error, Recovery, Non-Functional scenarios + - For each scenario class, ask: "Are [scenario type] requirements complete, clear, and consistent?" + - If scenario class missing: "Are [scenario type] requirements intentionally excluded or missing? [Gap]" + - Include resilience/rollback when state mutation occurs: "Are rollback requirements defined for migration failures? [Gap]" + + **Traceability Requirements**: + - MINIMUM: ≥80% of items MUST include at least one traceability reference + - Each item should reference: spec section `[Spec §X.Y]`, or use markers: `[Gap]`, `[Ambiguity]`, `[Conflict]`, `[Assumption]` + - If no ID system exists: "Is a requirement & acceptance criteria ID scheme established? [Traceability]" + + **Surface & Resolve Issues** (Requirements Quality Problems): + Ask questions about the requirements themselves: + - Ambiguities: "Is the term 'fast' quantified with specific metrics? [Ambiguity, Spec §NFR-1]" + - Conflicts: "Do navigation requirements conflict between §FR-10 and §FR-10a? [Conflict]" + - Assumptions: "Is the assumption of 'always available podcast API' validated? [Assumption]" + - Dependencies: "Are external podcast API requirements documented? [Dependency, Gap]" + - Missing definitions: "Is 'visual hierarchy' defined with measurable criteria? [Gap]" + + **Content Consolidation**: + - Soft cap: If raw candidate items > 40, prioritize by risk/impact + - Merge near-duplicates checking the same requirement aspect + - If >5 low-impact edge cases, create one item: "Are edge cases X, Y, Z addressed in requirements? [Coverage]" + + **🚫 ABSOLUTELY PROHIBITED** - These make it an implementation test, not a requirements test: + - ❌ Any item starting with "Verify", "Test", "Confirm", "Check" + implementation behavior + - ❌ References to code execution, user actions, system behavior + - ❌ "Displays correctly", "works properly", "functions as expected" + - ❌ "Click", "navigate", "render", "load", "execute" + - ❌ Test cases, test plans, QA procedures + - ❌ Implementation details (frameworks, APIs, algorithms) + + **✅ REQUIRED PATTERNS** - These test requirements quality: + - ✅ "Are [requirement type] defined/specified/documented for [scenario]?" + - ✅ "Is [vague term] quantified/clarified with specific criteria?" + - ✅ "Are requirements consistent between [section A] and [section B]?" + - ✅ "Can [requirement] be objectively measured/verified?" + - ✅ "Are [edge cases/scenarios] addressed in requirements?" + - ✅ "Does the spec define [missing aspect]?" + +7. **Structure Reference**: Generate the checklist following the canonical template in `.specify/templates/checklist-template.md` for title, meta section, category headings, and ID formatting. If template is unavailable, use: H1 title, purpose/created meta lines, `##` category sections containing `- [ ] CHK### ` lines with globally incrementing IDs starting at CHK001. + +8. **Report**: Output full path to checklist file, item count, and summarize whether the run created a new file or appended to an existing one. Summarize: + - Focus areas selected + - Depth level + - Actor/timing + - Any explicit user-specified must-have items incorporated + +**Important**: Each `/speckit-checklist` command invocation uses a short, descriptive checklist filename and either creates a new file or appends to an existing one. This allows: + +- Multiple checklists of different types (e.g., `ux.md`, `test.md`, `security.md`) +- Simple, memorable filenames that indicate checklist purpose +- Easy identification and navigation in the `checklists/` folder + +To avoid clutter, use descriptive types and clean up obsolete checklists when done. + +## Example Checklist Types & Sample Items + +**UX Requirements Quality:** `ux.md` + +Sample items (testing the requirements, NOT the implementation): + +- "Are visual hierarchy requirements defined with measurable criteria? [Clarity, Spec §FR-1]" +- "Is the number and positioning of UI elements explicitly specified? [Completeness, Spec §FR-1]" +- "Are interaction state requirements (hover, focus, active) consistently defined? [Consistency]" +- "Are accessibility requirements specified for all interactive elements? [Coverage, Gap]" +- "Is fallback behavior defined when images fail to load? [Edge Case, Gap]" +- "Can 'prominent display' be objectively measured? [Measurability, Spec §FR-4]" + +**API Requirements Quality:** `api.md` + +Sample items: + +- "Are error response formats specified for all failure scenarios? [Completeness]" +- "Are rate limiting requirements quantified with specific thresholds? [Clarity]" +- "Are authentication requirements consistent across all endpoints? [Consistency]" +- "Are retry/timeout requirements defined for external dependencies? [Coverage, Gap]" +- "Is versioning strategy documented in requirements? [Gap]" + +**Performance Requirements Quality:** `performance.md` + +Sample items: + +- "Are performance requirements quantified with specific metrics? [Clarity]" +- "Are performance targets defined for all critical user journeys? [Coverage]" +- "Are performance requirements under different load conditions specified? [Completeness]" +- "Can performance requirements be objectively measured? [Measurability]" +- "Are degradation requirements defined for high-load scenarios? [Edge Case, Gap]" + +**Security Requirements Quality:** `security.md` + +Sample items: + +- "Are authentication requirements specified for all protected resources? [Coverage]" +- "Are data protection requirements defined for sensitive information? [Completeness]" +- "Is the threat model documented and requirements aligned to it? [Traceability]" +- "Are security requirements consistent with compliance obligations? [Consistency]" +- "Are security failure/breach response requirements defined? [Gap, Exception Flow]" + +## Anti-Examples: What NOT To Do + +**❌ WRONG - These test implementation, not requirements:** + +```markdown +- [ ] CHK001 - Verify landing page displays 3 episode cards [Spec §FR-001] +- [ ] CHK002 - Test hover states work correctly on desktop [Spec §FR-003] +- [ ] CHK003 - Confirm logo click navigates to home page [Spec §FR-010] +- [ ] CHK004 - Check that related episodes section shows 3-5 items [Spec §FR-005] +``` + +**✅ CORRECT - These test requirements quality:** + +```markdown +- [ ] CHK001 - Are the number and layout of featured episodes explicitly specified? [Completeness, Spec §FR-001] +- [ ] CHK002 - Are hover state requirements consistently defined for all interactive elements? [Consistency, Spec §FR-003] +- [ ] CHK003 - Are navigation requirements clear for all clickable brand elements? [Clarity, Spec §FR-010] +- [ ] CHK004 - Is the selection criteria for related episodes documented? [Gap, Spec §FR-005] +- [ ] CHK005 - Are loading state requirements defined for asynchronous episode data? [Gap] +- [ ] CHK006 - Can "visual hierarchy" requirements be objectively measured? [Measurability, Spec §FR-001] +``` + +**Key Differences:** + +- Wrong: Tests if the system works correctly +- Correct: Tests if the requirements are written correctly +- Wrong: Verification of behavior +- Correct: Validation of requirement quality +- Wrong: "Does it do X?" +- Correct: "Is X clearly specified?" + +## Post-Execution Checks + +**Check for extension hooks (after checklist generation)**: +Check if `.specify/extensions.yml` exists in the project root. +- If it exists, read it and look for entries under the `hooks.after_checklist` key +- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally +- Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. +- For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: + - If the hook has no `condition` field, or it is null/empty, treat the hook as executable + - If the hook defines a non-empty `condition`, skip the hook and leave condition evaluation to the HookExecutor implementation +- When constructing slash commands from hook command names, replace dots (`.`) with hyphens (`-`). For example, `speckit.git.commit` → `/speckit-git-commit`. +- For each executable hook, output the following based on its `optional` flag: + - **Optional hook** (`optional: true`): + ``` + ## Extension Hooks + + **Optional Hook**: {extension} + Command: `/{command}` + Description: {description} + + Prompt: {prompt} + To execute: `/{command}` + ``` + - **Mandatory hook** (`optional: false`): + ``` + ## Extension Hooks + + **Automatic Hook**: {extension} + Executing: `/{command}` + EXECUTE_COMMAND: {command} + ``` + After emitting the block above you MUST actually invoke the hook and wait for it to finish before continuing. Run it the same way you would run the command yourself in this agent/session (the invocation may differ from the literal `{command}` id shown above, e.g. a skills-mode agent runs it as `/skill:speckit-...` or `$speckit-...`). Emitting the block alone does not run the hook. +- If no hooks are registered or `.specify/extensions.yml` does not exist, skip silently diff --git a/.claude/skills/speckit-clarify/SKILL.md b/.claude/skills/speckit-clarify/SKILL.md new file mode 100644 index 000000000..1715752e7 --- /dev/null +++ b/.claude/skills/speckit-clarify/SKILL.md @@ -0,0 +1,288 @@ +--- +name: "speckit-clarify" +description: "Identify underspecified areas in the current feature spec by asking up to 5 highly targeted clarification questions and encoding answers back into the spec." +argument-hint: "Optional areas to clarify in the spec" +compatibility: "Requires spec-kit project structure with .specify/ directory" +metadata: + author: "github-spec-kit" + source: "templates/commands/clarify.md" +user-invocable: true +disable-model-invocation: false +--- + + +## User Input + +```text +$ARGUMENTS +``` + +You **MUST** consider the user input before proceeding (if not empty). + +## Pre-Execution Checks + +**Check for extension hooks (before clarification)**: +- Check if `.specify/extensions.yml` exists in the project root. +- If it exists, read it and look for entries under the `hooks.before_clarify` key +- If the YAML cannot be parsed or is invalid, skip hook checking silently and continue normally +- Filter out hooks where `enabled` is explicitly `false`. Treat hooks without an `enabled` field as enabled by default. +- For each remaining hook, do **not** attempt to interpret or evaluate hook `condition` expressions: + - If the hook has no `condition` field, or it is null/empty, treat the hook as executable + - If the hook defines a non-empty `condition`, skip the hook and leave condition evaluation to the HookExecutor implementation +- When constructing slash commands from hook command names, replace dots (`.`) with hyphens (`-`). For example, `speckit.git.commit` → `/speckit-git-commit`. +- For each executable hook, output the following based on its `optional` flag: + - **Optional hook** (`optional: true`): + ``` + ## Extension Hooks + + **Optional Pre-Hook**: {extension} + Command: `/{command}` + Description: {description} + + Prompt: {prompt} + To execute: `/{command}` + ``` + - **Mandatory hook** (`optional: false`): + ``` + ## Extension Hooks + + **Automatic Pre-Hook**: {extension} + Executing: `/{command}` + EXECUTE_COMMAND: {command} + + Wait for the result of the hook command before proceeding to the Outline. + ``` + After emitting the block above you MUST actually invoke the hook and wait for it to finish before continuing. Run it the same way you would run the command yourself in this agent/session (the invocation may differ from the literal `{command}` id shown above, e.g. a skills-mode agent runs it as `/skill:speckit-...` or `$speckit-...`). Emitting the block alone does not run the hook. +- If no hooks are registered or `.specify/extensions.yml` does not exist, skip silently + +## Outline + +Goal: Detect and reduce ambiguity or missing decision points in the active feature specification and record the clarifications directly in the spec file. + +Note: This clarification workflow is expected to run (and be completed) BEFORE invoking `/speckit-plan`. If the user explicitly states they are skipping clarification (e.g., exploratory spike), you may proceed, but must warn that downstream rework risk increases. + +Execution steps: + +1. Run `.specify/scripts/bash/check-prerequisites.sh --json --paths-only` from repo root **once** (combined `--json --paths-only` mode / `-Json -PathsOnly`). Parse minimal JSON payload fields: + - `FEATURE_DIR` + - `FEATURE_SPEC` + - (Optionally capture `IMPL_PLAN`, `TASKS` for future chained flows.) + - If JSON parsing fails, abort and instruct user to re-run `/speckit-specify` or verify feature branch environment. + - For single quotes in args like "I'm Groot", use escape syntax: e.g 'I'\''m Groot' (or double-quote if possible: "I'm Groot"). + +2. **IF EXISTS**: Load `.specify/memory/constitution.md` for project principles and governance constraints. + +3. Load the current spec file. Perform a structured ambiguity & coverage scan using this taxonomy. For each category, mark status: Clear / Partial / Missing. Produce an internal coverage map used for prioritization (do not output raw map unless no questions will be asked). + + Functional Scope & Behavior: + - Core user goals & success criteria + - Explicit out-of-scope declarations + - User roles / personas differentiation + + Domain & Data Model: + - Entities, attributes, relationships + - Identity & uniqueness rules + - Lifecycle/state transitions + - Data volume / scale assumptions + + Interaction & UX Flow: + - Critical user journeys / sequences + - Error/empty/loading states + - Accessibility or localization notes + + Non-Functional Quality Attributes: + - Performance (latency, throughput targets) + - Scalability (horizontal/vertical, limits) + - Reliability & availability (uptime, recovery expectations) + - Observability (logging, metrics, tracing signals) + - Security & privacy (authN/Z, data protection, threat assumptions) + - Compliance / regulatory constraints (if any) + + Integration & External Dependencies: + - External services/APIs and failure modes + - Data import/export formats + - Protocol/versioning assumptions + + Edge Cases & Failure Handling: + - Negative scenarios + - Rate limiting / throttling + - Conflict resolution (e.g., concurrent edits) + + Constraints & Tradeoffs: + - Technical constraints (language, storage, hosting) + - Explicit tradeoffs or rejected alternatives + + Terminology & Consistency: + - Canonical glossary terms + - Avoided synonyms / deprecated terms + + Completion Signals: + - Acceptance criteria testability + - Measurable Definition of Done style indicators + + Misc / Placeholders: + - TODO markers / unresolved decisions + - Ambiguous adjectives ("robust", "intuitive") lacking quantification + + For each category with Partial or Missing status, add a candidate question opportunity unless: + - Clarification would not materially change implementation or validation strategy + - Information is better deferred to planning phase (note internally) + +4. Generate (internally) a prioritized queue of candidate clarification questions (maximum 5). Do NOT output them all at once. Apply these constraints: + - Maximum of 5 total questions across the whole session. + - Each question must be answerable with EITHER: + - A short multiple‑choice selection (2–5 distinct, mutually exclusive options), OR + - A one-word / short‑phrase answer (explicitly constrain: "Answer in <=5 words"). + - Only include questions whose answers materially impact architecture, data modeling, task decomposition, test design, UX behavior, operational readiness, or compliance validation. + - Ensure category coverage balance: attempt to cover the highest impact unresolved categories first; avoid asking two low-impact questions when a single high-impact area (e.g., security posture) is unresolved. + - Exclude questions already answered, trivial stylistic preferences, or plan-level execution details (unless blocking correctness). + - Favor clarifications that reduce downstream rework risk or prevent misaligned acceptance tests. + - If more than 5 categories remain unresolved, select the top 5 by (Impact * Uncertainty) heuristic. + +5. Sequential questioning loop (interactive): + - Present EXACTLY ONE question at a time. + - For multiple‑choice questions: + - **Analyze all options** and determine the **most suitable option** based on: + - Best practices for the project type + - Common patterns in similar implementations + - Risk reduction (security, performance, maintainability) + - Alignment with any explicit project goals or constraints visible in the spec + - Present your **recommended option prominently** at the top with clear reasoning (1-2 sentences explaining why this is the best choice). + - Format as: `**Recommended:** Option [X] - ` + - Then render all options as a Markdown table: + + | Option | Description | + |--------|-------------| + | A |