diff --git a/context/skills/audit-feature-flags/config.yaml b/context/skills/audit-feature-flags/config.yaml index a76ec32e..580cd9a2 100644 --- a/context/skills/audit-feature-flags/config.yaml +++ b/context/skills/audit-feature-flags/config.yaml @@ -1,20 +1,19 @@ type: skill template: description.md -description: Audit a PostHog integration's feature flag usage for correctness and cost-optimization opportunities +description: Verify, diagnose, and fix a PostHog feature flags integration — live delivery checks plus correctness and cost audits, with consent-gated fixes tags: [best-practices] cli: role: command parentCommand: audit command: feature-flags -references: - preamble: "**Read ONLY this file.** Do not read any other reference file until this one tells you to." shared_docs: - https://posthog.com/docs/feature-flags/best-practices.md - https://posthog.com/docs/feature-flags/cutting-costs.md - https://posthog.com/docs/feature-flags/local-evaluation.md - https://posthog.com/docs/feature-flags/bootstrapping.md + - https://posthog.com/docs/feature-flags/cleaning-up-stale-flags.md variants: - id: all - display_name: PostHog audit — feature flags + display_name: Feature flags doctor tags: [best-practices] docs_urls: [] diff --git a/context/skills/audit-feature-flags/description.md b/context/skills/audit-feature-flags/description.md index 39232516..dde7323e 100644 --- a/context/skills/audit-feature-flags/description.md +++ b/context/skills/audit-feature-flags/description.md @@ -1,74 +1,174 @@ -# PostHog Audit — Feature Flags +# PostHog Feature Flags Doctor -This skill audits an existing PostHog integration's **feature flag usage** for both correctness (fix) and cost-optimization (optimize). **Read-only** — the only file you create is the final audit report. The audit never mutates PostHog state and never edits project source. +This skill checks an existing PostHog project's **feature flags** end to end and fixes the problems the user chooses. It goes beyond static analysis: it verifies flags are actually **delivered** to the app by probing the same `/flags` endpoint the SDK uses, cross-checks the response against the flag definitions in PostHog, and diagnoses the silent failure modes static analysis cannot see. It works in three phases: -The audit covers two lenses: +1. **Verify (read-only):** static correctness and cost checks over the source tree, plus live delivery and observability checks against the project's real `/flags` endpoint and flag roster. +2. **Confirm:** present the findings and let the user pick which ones to fix — a single `wizard_ask` multi-select. +3. **Fix:** apply only the selected fixes — to the user's code and/or their PostHog project via the MCP — then write a report. -- **Fix** — correctness checks on flag bootstrapping, readiness/race conditions, and default-value fallbacks. These prevent flicker, mis-evaluation, and runtime errors when flags fail to load. -- **Optimize** — cost-side checks on unreferenced active flags, local-evaluation polling interval, local evaluation inside edge/Lambda handlers, and test/CI gating of flag fetches. These use PostHog MCP to read the operator's tenant where applicable; they gracefully skip if MCP is unavailable. +The verify phase never changes anything. Changes happen only in the fix phase, and only for findings the user explicitly selected. Output: a markdown report at `posthog-feature-flags-report.md` at the project root. -The billed feature flag endpoint is `/flags` (the renamed `/decide`). All references to the "decide endpoint" in older docs map to `/flags`. +## Reference files -## Workflow +{references} -The audit runs as a step chain. **The exact step list lives in the reference files themselves, not in this overview.** Step 1 lives at `references/1-presence.md`; each step file ends with a `next_step:` frontmatter pointer to the next, and the final step has `next_step: null`. Follow them in the order they point. You must resolve each step in order before any source-tree exploration. +**Read discipline.** Read each reference at the phase that needs it, not before: `references/checks.md` when Phase 1 begins; `references/remediation.md` at Phase 2 (and again in Phase 3 when applying fixes); `references/report-format.md` when writing the report. The `prompt-ff-*.md` files are subagent prompts — each dispatched subagent reads exactly its own, and the main loop never reads them. Do not `Glob`/`ls` the skill directory or preload reference files; the fetched doc files (`best-practices.md`, `bootstrapping.md`, `cutting-costs.md`, and the rest) are for subagents and remediation links — never read them wholesale from the main loop. -The audit ledger is seeded by the wizard with one pending check per feature flag check. **Each step gracefully handles a missing check id**: if a step's expected id is not in the ledger, it skips its `audit_resolve_checks` call for that id and continues. Use `mcp__wizard-tools__audit_resolve_checks` to patch each check as you finish it. +## Guiding tenets -**Start by reading the path relative to this file at `references/1-presence.md`.** Do not Glob, ls, or find the skill directory. Do not preload future steps. Do not re-read a step file once you've moved past it. Do not re-read SKILL.md. +1. **Consent-gated changes.** The verify phase is read-only. In the fix phase, change only what the user selected in the `wizard_ask` step — never touch a finding they didn't pick, and never make a change beyond the remediation mapped for that finding in `references/remediation.md`. -`ToolSearch` is only for loading a tool by exact name when the SDK has it deferred (e.g. `select:Grep`). Do **not** use it to browse for other tools — every tool the audit needs (`Glob`, `Grep`, `Read`, `Write`, `Bash`, the named `mcp__wizard-tools__audit_*` tools) is already named in this skill. The optimize-side checks reach PostHog through its single `exec` tool, described in the optimize reference. +2. **The cleanup interlock.** PostHog measures flag staleness by `$feature_flag_called` events. If this project evaluates flags but does not send those events (`ff-evaluated-not-reported` resolves as a finding), a flag in heavy production use is indistinguishable from a dead one. In that state, NEVER offer archive/disable fixes for tenant-side flags — offer only the fix that restores evaluation reporting, and record the cleanup candidates under "Manual follow-up" with an explanation. This is the most important correctness rule in this skill. -**Do not call `TaskCreate` / `TaskUpdate` / `TaskGet` / `TaskList`.** The audit doesn't track its own task list — progress comes from the audit ledger plus `[STATUS]` lines. +3. **Evidence-based.** Every non-pass finding cites `file:line` for code findings, or the probe request/response facts (status code, flag counts, `reason` codes) for live findings. Never fabricate or estimate; report only what greps, probes, and MCP calls actually returned. + +4. **Expected behavior is taught, not reported.** PostHog intentionally filters automated clients (headless browsers, crawlers) from `/flags` — such clients receive `{"errorsWhileComputingFlags": false, "flags": {}}` by design. This is a teaching callout in the report, never a finding. It also constrains the doctor itself: **every `/flags` probe must send a realistic browser User-Agent** (see `references/checks.md`), or the probe manufactures a false failure. + +5. **Secrets stay secret.** The project API key (`phc_…`) is a public client token and may be used in probe commands, but never paste personal API keys (`phx_…`) into commands, details, or the report. Never edit `.env` directly — use the wizard-tools MCP (`check_env_keys` / `set_env_values`) for environment values. Never include tokens or PII in the report. + +6. **Ledger contract.** When run by the wizard's native program, the audit ledger is pre-seeded — patch rows with `mcp__wizard-tools__audit_resolve_checks` and append sweep rows with `mcp__wizard-tools__audit_add_checks`. NEVER call `audit_seed_checks` (it atomically replaces the ledger and wipes the seeded rows). Every ledger call gracefully handles a missing check id: if an expected id is not in the ledger, skip its resolve call and continue — the run may be a plain skill run with no ledger at all. + +## Available tools + +{{> mcp-tool-calling}} + +**Verify (read-only):** +- `Glob` / `Grep` / `Read` — static checks over the source tree. +- `Bash` (plain `curl` only) — live `/flags` probes. Keep probe commands minimal and legible; see `references/checks.md` for the exact shapes. +- `feature-flag-get-all` (or the equivalent MCP flag-listing tool; `execute-sql` fallback) — the project's flag roster. +- `mcp__wizard-tools__check_env_keys` — which env keys exist (never reveals values). +- `docs-search` — latest doc URLs for remediation links. + +**Confirm:** +- `mcp__wizard-tools__wizard_ask` — ask which findings to fix. Call it **once** with a single multi-select question (see Phase 2). + +**Fix:** +- `Read` and `Edit` — apply code fixes. Always Read a file immediately before editing it. +- The MCP flag-mutation tool (e.g. `feature-flag-update` or equivalent) — archive/disable flags the user selected. If no such tool is available, do NOT guess a tool name: record the fix as manual guidance instead. +- `mcp__wizard-tools__set_env_values` — environment fixes. ## Live activity — `[STATUS]` -The "Working on …" banner reads from `[STATUS]` lines you emit in plain text. Whenever you start a new sub-step, write a line like: +The "Working on …" banner reads from `[STATUS]` lines you emit in plain text. Emit one whenever you start a new sub-step: ``` -[STATUS] Scanning feature flag call sites +[STATUS] Probing /flags delivery ``` -The wizard intercepts these and updates the spinner. Use them freely — they are cheap. Each step file lists the exact `[STATUS]` strings to emit at each sub-step. +The full list of expected `[STATUS]` lines is in the Status section below and per-check in `references/checks.md`. ## Audit checks ledger -The ledger lives at `.posthog-audit-checks.json` and is rendered live in the "Audit plan" tab. It is owned by MCP tools — **never `Write` this file directly**: +The ledger lives at `.posthog-audit-checks.json` and renders live in the wizard's "Audit plan" tab. It is owned by MCP tools — **never `Write` this file directly**: + +- `mcp__wizard-tools__audit_resolve_checks({ updates })` — patch checks by `id`: `{ id, status, file?, details? }`. Batch updates from the same step into one call. Errors on unknown ids — skip ids that aren't in the ledger. +- `mcp__wizard-tools__audit_add_checks({ checks })` — append sweep rows (per-flag findings that can't be enumerated at seed time). Never call it with an empty array (rejected). Prefix appended ids (`delivered-`, `ghost-`, `stale-`) and de-duplicate — one duplicate id rejects the whole batch. + +Seeded check ids (the wizard's native program seeds these; a plain skill run may have none): `ff-presence`, `ff-key-authenticates`, `ff-flags-endpoint`, `ff-flags-delivered`, `ff-unknown-flags`, `ff-evaluated-not-reported`, `ff-bootstrap-when-known-set`, `ff-await-readiness`, `ff-default-values`, `ff-bootstrap-distinct-id-mismatch`, `ff-identified-only-pre-auth-targeting`, `ff-eval-before-identify`, `ff-active-but-unreferenced`, `ff-stale-rolled-out`, `ff-local-eval-polling-interval`, `ff-local-eval-in-edge-handlers`, `ff-test-ci-gating`, `apply-fixes`, `write-report`. + +Check areas (group headings in the plan tab and report): `Feature Flags — Delivery`, `Feature Flags — Observability`, `Feature Flags` (correctness), `Feature Flags — Optimize` (cost), `Workflow`. + +Statuses: `pending` | `pass` | `error` | `warning` | `suggestion`. Severity meanings: +- `error`: must fix — broken functionality or guaranteed wrong behavior. +- `warning`: should fix — a pattern that causes subtle bugs, wrong data, or silent failure. +- `suggestion`: nice to have — best-practice or cost-savings opportunity. + +After the report is written, delete `.posthog-audit-checks.json` if it exists. + +## Pre-flight + +Emit `[STATUS] Detecting PostHog feature flag usage`, then run two `Grep` calls in parallel (`output_mode: "files_with_matches"`): -- `mcp__wizard-tools__audit_resolve_checks({ updates })` — patch one or more checks by `id`. Each `update` is `{ id, status, file?, details? }`. Batch updates from the same step into a single call. +1. Flag API surface: `getFeatureFlag|isFeatureEnabled|useFeatureFlag|onFeatureFlags|reloadFeatureFlags|getFeatureFlagPayload|featureFlags\.|posthog\.feature_enabled` +2. Local-evaluation signals: `personal_api_key|getAllFlagsAndPayloads|getAllFlags` -All audit ledger calls are atomic and serialize internally — **concurrent calls from parallel subagents cannot lose updates**, so feel free to fan out runtime checks across `Agent` subagents when a step says so. +Decision: +- Surface grep has zero hits AND the PostHog SDK is absent from the project (no `posthog-js`/`posthog-node`/etc. in a package manifest, no `posthog.init(`): emit `[ABORT] PostHog SDK not installed` and stop. +- Surface grep has zero hits but the SDK is present: emit `[ABORT] No feature flag usage` and stop. +- If flag-roster MCP calls later fail with a permissions error: emit `[ABORT] Insufficient permissions` and stop. -### Check entry shape +The wizard catches `[ABORT]` and terminates the run cleanly — do not halt yourself. -- `id` — stable kebab-case slug. Reuse the existing seeded ids exactly when calling `audit_resolve_checks`. -- `area` — short group name. This skill seeds two areas: `Feature Flags` (fix) and `Feature Flags — Optimize` (cost). -- `label` — short human name. -- `status` — `pending` | `pass` | `error` | `warning` | `suggestion`. -- `file` — optional `path:line` for findings tied to a location. -- `details` — optional one-line explanation. +Record for later: whether **local evaluation** is in use (second grep has ≥1 hit) — it gates two optimize checks; and resolve `ff-presence` as `pass` with the call-site count in `details`. -After the final step writes the report, delete `.posthog-audit-checks.json`. +## Phase 1 — Verify (read-only) -## Severity levels +Run the checks in `references/checks.md`, in this order: -- `error`: Must fix. Broken functionality, data corruption, or security issue. -- `warning`: Should fix. Pattern that causes subtle bugs or data-quality problems. -- `suggestion`: Nice to have. Best-practice improvement or cost-savings opportunity. +1. **Static checks** (correctness + cost) — dispatch the parallel subagents exactly as specified. These need no credentials. +2. **Live checks** (delivery + observability) — run after the static fan-out returns. These use the project token, the `/flags` endpoint, and the MCP flag roster. If credentials or MCP are unavailable, each live check degrades as its own section specifies (resolve as `suggestion` with a `details` explanation — never block the audit). -## Key principles +This phase is strictly read-only: `Grep`/`Read`/`Glob`, plain `curl` probes, and read-only MCP calls only. Do not modify anything yet. -- **Read-only**: Do not edit project source files. The only file you create is the audit report. -- **Evidence-based**: Reference specific `file:line` for every non-pass finding. For MCP-based optimize checks, include the SQL or MCP call summary in `details`. -- **Actionable**: Every finding states what to fix and how. -- **Graceful MCP fallback**: When PostHog MCP is unavailable, optimize checks resolve as `suggestion` with `details: "PostHog MCP unavailable — could not measure X"` and `mcp_skipped: true`. Do not block the audit. -- **Local-evaluation gating**: Some optimize checks only apply when the project uses server-side local evaluation (initialized with a `personal_api_key` / feature-flags secure API key, or calling local-eval-only APIs). Step 1 detects this and downstream checks skip themselves when local eval is not in use. +## Phase 2 — Confirm which fixes to apply + +Classify each finding by fix type using `references/remediation.md`: `code`, `settings`, or `manual`. Only `code` and `settings` findings are auto-fixable; `manual` findings go to the report's "Manual follow-up" section. + +**Apply the interlock first (tenet 2):** if `ff-evaluated-not-reported` resolved as `warning` or `error`, remove every tenant-side archive/disable option (`settings` fixes derived from `ff-active-but-unreferenced` or `ff-stale-rolled-out`) from the multi-select. Keep the fix that restores evaluation reporting. Explain the gating in the prompt text ("cleanup suggestions are withheld until evaluation events are verified — see report"). + +- If there are **no findings**, skip the confirm step, write a clean-bill report, and stop. +- If findings exist but **none are fixable** (all `manual`), skip the confirm step, write the report with them under "Manual follow-up", and stop. +- Otherwise call `mcp__wizard-tools__wizard_ask` **exactly once**: + - `kind: "multi"`. + - `prompt`: short summary, e.g. "Found N feature flag issues. Select the ones you'd like me to fix:". Mention manual-only findings are in the report. + - `options`: one `{ label, value }` per fixable finding. `value`: the check id (or `":"` for per-flag rows). + - **`label` is ONE short line — 80 characters or fewer, no newlines.** `wizard_ask` options have no description field, so a multi-line label is a wall of text in the overlay. Format: `"[] — "`. Reasoning, evidence, and file:line detail belong in the report, never here. If the run is asking through a harness-native question tool instead (e.g. the Anthropic harness's `AskUserQuestion`, whose options take an optional `description`), keep the label just as short and put a single line of detail in `description` — the report still owns the full reasoning. + - Good: `"[SUGGESTION] Fix typo'd flag key beta-serach → beta-search — src/nav.tsx:41"` + - Good: `"[WARNING] Send evaluation events for promo-banner — promo-banner.tsx:16"` + - Bad: any label that explains *why*, spans lines, or restates the check's rationale. + - **Options are fixes the doctor will apply — nothing else.** Never include "Skip", "I'll do it manually", "No action needed", or any informational/placeholder option: selecting nothing already skips everything, and `manual` findings live in the report, not the multi-select. Every option, when selected, must map to a concrete `code` or `settings` remediation from `references/remediation.md`. +- If `wizard_ask` returns an error (non-interactive host / CI), do NOT fail: skip the fix phase, write the report with all findings under "Manual follow-up", and stop. +- The user may select nothing — apply nothing and write the report. + +Emit `[STATUS] Asking which fixes to apply` before the call. + +## Phase 3 — Apply the selected fixes + +For each selected finding (and only those), apply the remediation mapped in `references/remediation.md`: + +- **code** fixes: locate the file with Grep/Read, then Edit. Read a file immediately before editing it. Minimal change for that finding only. +- **settings** fixes: apply via the MCP flag-mutation tool if available; otherwise record as manual guidance. +- Respect the safe cleanup order (remediation.md): flags still referenced in code are NEVER disabled/archived in PostHog by the doctor — code gate first, then the user deploys, then they disable. Only zero-reference flags may be archived directly, and only when the interlock allows. +- Track what changed per fix (file or flag), so "Fixes applied" is accurate. +- Resolve `apply-fixes` in the ledger when done (pass if all selected fixes applied). + +Emit `[STATUS] Applying fix: ` before each fix. + +## Output + +Emit `[STATUS] Writing report`, then write `posthog-feature-flags-report.md` to the project root following `references/report-format.md`, and display the report contents in chat as plain markdown (no wrapper commentary). Resolve `write-report` in the ledger, delete `.posthog-audit-checks.json` if present, output one final line confirming the report path, and emit `[STATUS] Done`. + +## Constraints + +- Modify code or PostHog state **only** for findings the user explicitly selected, and only the change mapped in `references/remediation.md`. +- Never archive/disable a flag that still has code references. Never offer tenant-side cleanup while the interlock is failing. +- Every `/flags` probe uses a realistic browser User-Agent and plain `curl`. No pipes to shells, no command substitution around secrets. +- Do NOT include PII or key values in the report — flag keys, hosts, paths, counts, and `reason` codes only. +- Do NOT call `TaskCreate` / `TaskUpdate` / `TaskGet` / `TaskList` — progress comes from the ledger and `[STATUS]` lines. + +## Status + +- Detecting PostHog feature flag usage +- Auditing feature flag correctness +- Auditing feature flag cost optimization +- Probing /flags delivery +- Cross-checking delivered flags against definitions +- Checking flag keys referenced in code exist in PostHog +- Verifying evaluation events are reported +- Asking which fixes to apply +- Applying fix: +- Writing report +- Done ## Abort statuses -Report abort states with `[ABORT]` prefixed messages. The wizard catches these and terminates the run — do not halt yourself. +Report abort states with `[ABORT]` prefixed messages — wording must match exactly so the wizard renders the right error UI: + +- `[ABORT] No feature flag usage` — no flag call sites found, but a PostHog SDK is present. +- `[ABORT] PostHog SDK not installed` — no flag call sites and the PostHog SDK is not present in the project. +- `[ABORT] Insufficient permissions` — the flag roster / query calls fail with a permissions error. -- No PostHog feature flag usage found (no SDK call sites matching the flag-eval API surface) +Stop all further work after emitting `[ABORT]`. ## Framework guidelines diff --git a/context/skills/audit-feature-flags/references/1-presence.md b/context/skills/audit-feature-flags/references/1-presence.md deleted file mode 100644 index 921d8a92..00000000 --- a/context/skills/audit-feature-flags/references/1-presence.md +++ /dev/null @@ -1,37 +0,0 @@ ---- -next_step: 2-feature-flags-fix.md ---- - -# Step 1 — Presence detector - -This step decides whether the rest of the audit has anything to look at, and records whether **server-side local evaluation** is in use (which gates several optimize checks). Run it **before** any other work. Resolve zero ledger checks here — this step is gating only. - -## Status - -Emit: - -``` -[STATUS] Detecting PostHog feature flag usage -``` - -## Action - -Run **two `Grep` calls in parallel**, both with `output_mode: "files_with_matches"`: - -1. Flag API surface — any of: - `getFeatureFlag|isFeatureEnabled|useFeatureFlag|onFeatureFlags|reloadFeatureFlags|getFeatureFlagPayload|featureFlags\.|posthog\.feature_enabled` -2. Local-evaluation signals — any of: - `personal_api_key|getAllFlagsAndPayloads|getAllFlags` - -## Decision - -- **Surface grep returns zero hits anywhere in the project:** emit `[ABORT] No PostHog feature flag usage found` and stop. The wizard catches `[ABORT]` and terminates the run. -- **Surface grep finds hits:** continue. - -## Record local-evaluation detection - -Local evaluation is detected when the second grep returns **at least one hit** (the project either initializes a server SDK with `personal_api_key` / a feature-flags secure API key, or calls a local-evaluation-only API like `getAllFlagsAndPayloads` / `getAllFlags` on the server). Keep this signal in working memory — Step 3 uses it to decide whether to run two of the optimize subagents or skip them as `pass` with `details: "skip: local evaluation not detected"`. - -Do not read any files in this step. Do not call `audit_resolve_checks`. Do not preload future steps. - -Continue to **`2-feature-flags-fix.md`**. diff --git a/context/skills/audit-feature-flags/references/2-feature-flags-fix.md b/context/skills/audit-feature-flags/references/2-feature-flags-fix.md deleted file mode 100644 index 9448f263..00000000 --- a/context/skills/audit-feature-flags/references/2-feature-flags-fix.md +++ /dev/null @@ -1,238 +0,0 @@ ---- -next_step: 3-feature-flags-optimize.md ---- - -# Step 2 — Feature flags (fix) - -This step resolves five correctness checks **in parallel**, one subagent per check: - -- `ff-bootstrap-when-known-set` -- `ff-await-readiness` -- `ff-default-values` -- `ff-bootstrap-distinct-id-mismatch` -- `ff-identified-only-pre-auth-targeting` - -## Status - -Emit before dispatching: - -``` -[STATUS] Auditing feature flag correctness -``` - -## Action — dispatch five subagents in one message - -Make **five `Agent` tool calls in a single message** so they run concurrently. Wait for all five to return, then continue to `3-feature-flags-optimize.md`. Do not run any other tools between dispatch and the next step. - -The bundled `best-practices.md` reference holds PostHog's authoritative guidance on flag bootstrapping, readiness, and default values. It's typically at `.claude/skills/audit-feature-flags/references/best-practices.md`; if that path doesn't exist, discover it with `Glob` `**/skills/audit-feature-flags/references/best-practices.md`. Each subagent reads it once before judging. - -### Task A — `ff-bootstrap-when-known-set` - -`description`: `Audit ff-bootstrap-when-known-set` - -`prompt`: -```` -You are an audit subagent. Resolve exactly one rule and return: ff-bootstrap-when-known-set. - -Read this skill's bundled `best-practices.md` reference once (typically `.claude/skills/audit-feature-flags/references/best-practices.md`; otherwise discover with `Glob` `**/skills/audit-feature-flags/references/best-practices.md`). Focus on the bootstrapping guidance — when an initial flag set is already known at app start (e.g. computed server-side, persisted in a cookie, or passed through SSR props), client-side `posthog.init` should set `bootstrap.featureFlags` so the first render has the right values without a `/flags` round trip. - -Run **two** Greps in parallel: -- `posthog\.init\(|new PostHog\(|posthog\.Posthog\(|Posthog\(` — every PostHog init site. -- `getFeatureFlag\(|isFeatureEnabled\(|useFeatureFlag\(` — every flag-eval call site. - -Read each file that contains an init hit, once. For each init, inspect the options object: is `bootstrap.featureFlags` (or `bootstrap: { featureFlags: ... }`) provided? - -Then check whether the codebase has a known initial flag set referenced **before init returns** — common signals: -- SSR / server-rendered props that pass flag values into a `` / init call. -- A `cookies` / `headers` read that yields flag values, used near init. -- An explicit constant or map of flag keys imported into the init module. -- Flag-eval call sites running synchronously inside the same render path that mounts the provider. - -Rule: -- pass: bootstrap is set when a known initial flag set exists, OR no known initial set is referenced before init (nothing to bootstrap with). -- warning: a known initial flag set is referenced before init returns but `bootstrap.featureFlags` is not set on init — early flag evals will return `undefined` and cause flicker. -- suggestion: init has neither bootstrap nor any `onFeatureFlags` / `loaded` callback gating early evals — recommend either bootstrap (preferred) or readiness gating. - -Emit one `mcp__wizard-tools__audit_resolve_checks` call with a single update for id `ff-bootstrap-when-known-set`, including `file` (path:line of the init that lacks bootstrap) and `details` as compact JSON: - -``` -{ - "init_call_count": , - "init_with_bootstrap_count": , - "known_initial_set_detected": true | false, - "examples": [ - {"file": "", "issue": "missing-bootstrap | no-readiness-gate"} - ] -} -``` - -Return when the call completes. Do not write the audit report. -```` - -### Task B — `ff-await-readiness` - -`description`: `Audit ff-await-readiness` - -`prompt`: -```` -You are an audit subagent. Resolve exactly one rule and return: ff-await-readiness. - -Read this skill's bundled `best-practices.md` reference once (typically `.claude/skills/audit-feature-flags/references/best-practices.md`; otherwise discover with `Glob` `**/skills/audit-feature-flags/references/best-practices.md`). Focus on the readiness / "have the value before you need it" section — client-side flag evaluation is async, so any flag-eval before `onFeatureFlags` fires (or before the `loaded` callback runs, or before `bootstrap.featureFlags` is set) returns `undefined`, which is **not** `false`. Misreading the loading gap is one of the most common flag bugs. - -Run **three** Greps in parallel: -- `getFeatureFlag\(|isFeatureEnabled\(|useFeatureFlag\(|getFeatureFlagPayload\(` — every flag-eval call site. -- `onFeatureFlags\(|posthog\.onFeatureFlags\(` — readiness subscribers. -- `bootstrap\s*:|loaded\s*:|loaded\s*\(` — bootstrap config and `loaded` callbacks on init. - -Read each file that contains a flag-eval hit, once. For each flag-eval call, determine whether it is gated against the loading window — i.e. it happens after `onFeatureFlags` fires, inside / after a `loaded` callback, after a bootstrap was provided, or behind a readiness guard the project defines itself. - -Rule: -- pass: every flag-eval call site is either bootstrapped, behind `onFeatureFlags` / `loaded` gating, or inside a code path that only runs post-init (e.g. a click handler). -- warning: one or more flag-eval calls run in a render-on-mount path (React render body, `useEffect` with empty deps, Vue `onMounted`) without bootstrap or readiness gating — race-condition risk. -- error: a flag-eval call's return value is compared with `===` / `!==` to a non-undefined value in a path that can run before flags load, and the codebase has no bootstrap and no readiness subscribe — guaranteed undefined-handling bug. - -Emit one `mcp__wizard-tools__audit_resolve_checks` call with a single update for id `ff-await-readiness`, including `file` (path:line of the most representative offending flag-eval call) and `details` as compact JSON: - -``` -{ - "flag_eval_call_count": , - "ungated_call_count": , - "bootstrap_present": true | false, - "readiness_subscriber_present": true | false, - "examples": [ - {"file": "", "issue": "race-on-mount | undefined-misread"} - ] -} -``` - -Return when the call completes. Do not write the audit report. -```` - -### Task C — `ff-default-values` - -`description`: `Audit ff-default-values` - -`prompt`: -```` -You are an audit subagent. Resolve exactly one rule and return: ff-default-values. - -Read this skill's bundled `best-practices.md` reference once (typically `.claude/skills/audit-feature-flags/references/best-practices.md`; otherwise discover with `Glob` `**/skills/audit-feature-flags/references/best-practices.md`). Focus on the "undefined is not false" and per-flag default guidance — `getFeatureFlag('key')` returns `undefined` during the loading window and may also return `undefined` when PostHog is unreachable or quota-limited. A per-flag default (via `?? 'control'`, a wrapper helper, or the SDK's `default_value`/`defaultValue` option when supported) controls what users see during these windows. - -Run **one** Grep: `getFeatureFlag\(|isFeatureEnabled\(|useFeatureFlag\(` — every flag-eval call site. - -Read each file that contains a hit, once. For each flag-eval call, classify whether the result is consumed with a default-value fallback: -- **explicit `??` / `||` fallback** on the call expression — fine. -- **wrapped in a helper** that supplies a default (e.g. `function useBetaFeature() { return posthog.isFeatureEnabled('beta') ?? false }`) — fine. -- **explicit `=== 'variant'` / `!== 'variant'` comparison** treated as the default-handling — fine *only if* the surrounding code path can tolerate `undefined` (i.e. the variant branch is the opt-in and the fallthrough is safe). -- **bare consumption** — the call result feeds into a conditional, prop, or render without a default — flag. - -Rule: -- pass: every flag-eval call site either has a per-flag default fallback or is consumed via a safe variant comparison. -- suggestion: 1–2 bare flag-eval call sites — low risk, recommend adding `?? `. -- warning: 3+ bare flag-eval call sites, OR any bare call in a code path the user always hits (top-level render, app shell). - -Emit one `mcp__wizard-tools__audit_resolve_checks` call with a single update for id `ff-default-values`, including `file` (path:line of the most representative bare call) and `details` as compact JSON: - -``` -{ - "flag_eval_call_count": , - "bare_consumption_count": , - "examples": [ - {"file": "", "issue": "no-default-fallback"} - ] -} -``` - -Return when the call completes. Do not write the audit report. -```` - -### Task D — `ff-bootstrap-distinct-id-mismatch` - -`description`: `Audit ff-bootstrap-distinct-id-mismatch` - -`prompt`: -```` -You are an audit subagent. Resolve exactly one rule and return: ff-bootstrap-distinct-id-mismatch. - -Read this skill's bundled `bootstrapping.md` reference once (typically `.claude/skills/audit-feature-flags/references/bootstrapping.md`; otherwise discover with `Glob` `**/skills/audit-feature-flags/references/bootstrapping.md`). - -Background: `bootstrap.distinctID` (or `bootstrap: { distinctID: ... }`) lets the host application seed the SDK's distinct_id at init time — usually for SSR/SSG scenarios where the server already knows the user. But if the value passed doesn't match either the user's eventual stable id (after `identify()`) or the SDK's natural anonymous id, it overrides the identity chain in ways that break later merges. Two failure modes: -1. `distinctID` set to a per-request random / session UUID — the SDK considers itself "already identified" with that UUID; the next `identify(realUserId)` is blocked from merging anonymous activity. -2. `distinctID` set to a known user id but the project ALSO calls `identify(differentId)` shortly after — the two ids race; whichever loses creates an orphan profile. - -Run **two** Greps in parallel: -- `bootstrap[\s\S]{0,40}distinctID|bootstrap[\s\S]{0,40}distinct_id|distinctID\s*:` — bootstrap-with-distinctID sites. -- `posthog\.identify\(` — every identify call (so the subagent can cross-reference). - -Read each file that contains a bootstrap.distinctID hit, once. For each site, determine: -- What value is being passed (literal, variable, request-scoped, randomly generated)? -- Is the same value later passed to `posthog.identify()`? If yes, that's the safe pattern (matching SSR hydration). -- Is the value request-scoped / per-render (e.g. `crypto.randomUUID()`, `Math.random()`, a Next.js per-request id)? If yes, this is the failure mode. - -Rule: -- pass: no `bootstrap.distinctID` usage detected, OR the bootstrapped value is stable across requests and matches the value passed to a later identify() call. -- warning: `bootstrap.distinctID` is set to a value that appears request-scoped, randomly generated, or otherwise volatile — the next identify() call will be blocked from merging anonymous activity. -- error: `bootstrap.distinctID` is set to one value and `posthog.identify()` is called immediately after with a DIFFERENT value on the same code path — orphan profile guaranteed. - -Emit one `mcp__wizard-tools__audit_resolve_checks` call with a single update for id `ff-bootstrap-distinct-id-mismatch`, including `file` (path:line of the bootstrap site) and `details` as compact JSON: - -``` -{ - "bootstrap_distinct_id_site_count": , - "examples": [ - {"file": "", "issue": "volatile-bootstrap-id | bootstrap-identify-mismatch | safe-ssr-hydration"} - ] -} -``` - -Return when the call completes. Do not write the audit report. -```` - -### Task E — `ff-identified-only-pre-auth-targeting` - -`description`: `Audit ff-identified-only-pre-auth-targeting` - -`prompt`: -```` -You are an audit subagent. Resolve exactly one rule and return: ff-identified-only-pre-auth-targeting. - -Read this skill's bundled `best-practices.md` reference once (typically `.claude/skills/audit-feature-flags/references/best-practices.md`; otherwise discover with `Glob` `**/skills/audit-feature-flags/references/best-practices.md`). - -Background: when `person_profiles: 'identified_only'` is set (the recommended default for most B2B SaaS), anonymous visitors don't create person profiles. If a feature flag targets users by person properties AND that flag is evaluated on a pre-auth surface (landing page, pricing page, signup form), the anonymous user has no person profile for the flag to evaluate against, so the flag silently returns its default value. The variant the operator intended to ship to "users in the EU" / "users on the Pro plan" never reaches anyone visiting before login. This is a silent failure — the flag appears to work for identified users but the anonymous-traffic branch quietly never fires. - -Run **three** Greps in parallel: -- `person_profiles\s*:|personProfiles\s*:` — locate the person_profiles setting. -- `getFeatureFlag\(|isFeatureEnabled\(|useFeatureFlag\(|getFeatureFlagPayload\(` — every flag-eval call site. -- `posthog\.identify\(` — every identify call (used to classify a surface as pre-auth or post-auth). - -Step 1 — read the file(s) containing `person_profiles` hits to determine the configured value. If unset, the posthog-js default is `'identified_only'`. Record `mode` as `identified_only`, `always`, `never`, or `unset (defaults to identified_only)`. - -Step 2 — if mode is NOT `identified_only` (or unset), resolve `pass` with `details: "skip: person_profiles is not identified_only"` and return. - -Step 3 — for each flag-eval call site, read the surrounding file once. Classify it as **pre-auth** if it lives in: landing pages, marketing routes, pricing pages, signup/login UI components that render before the user authenticates, public homepage components, or any route gated to anonymous-only access. Classify as **post-auth** if the file also calls `posthog.identify()` in the same flow, requires authenticated session via middleware, or lives under a `/(app)/`, `/dashboard/`, `/(authenticated)/` style route. - -Step 4 — for each pre-auth flag-eval site, attempt to determine whether the flag's targeting condition references person properties. The skill can't read PostHog flag definitions; instead, flag any pre-auth eval whose flag key suggests person-property targeting (variants gated on plan, country, persona, role, signup_method, etc.) — name patterns like `eu-banner`, `pro-only-cta`, `enterprise-pricing-variant`. When ambiguous, default to warning and let the operator confirm. - -Rule: -- pass: mode is not identified_only, OR no flag-eval call sites run on pre-auth surfaces, OR all pre-auth flag evals pass property overrides at eval time (`getFeatureFlag(key, { personProperties: {...} })` or equivalent). -- suggestion: 1–2 pre-auth flag-eval call sites exist but flag names don't strongly suggest person-property targeting — recommend the operator confirm flag definitions in PostHog. -- warning: 3+ pre-auth flag-eval call sites OR any pre-auth flag-eval whose flag name strongly suggests person-property targeting — anonymous users silently get default values. Recommend either passing property overrides at eval time, switching to `posthog.bootstrap.featureFlags` with server-computed values, or moving the eval behind authentication. - -Emit one `mcp__wizard-tools__audit_resolve_checks` call with a single update for id `ff-identified-only-pre-auth-targeting`, including `file` (path:line of the most representative pre-auth flag-eval) and `details` as compact JSON: - -``` -{ - "person_profiles_mode": "identified_only | always | never | unset", - "pre_auth_flag_eval_count": , - "examples": [ - {"file": "", "flag_key": "", "suspected_property_targeting": } - ] -} -``` - -Return when the call completes. Do not write the audit report. -```` - -## After all five return - -Continue to **`3-feature-flags-optimize.md`**. diff --git a/context/skills/audit-feature-flags/references/3-feature-flags-optimize.md b/context/skills/audit-feature-flags/references/3-feature-flags-optimize.md deleted file mode 100644 index e1f23d21..00000000 --- a/context/skills/audit-feature-flags/references/3-feature-flags-optimize.md +++ /dev/null @@ -1,196 +0,0 @@ ---- -next_step: 4-report.md ---- - -# Step 3 — Feature flags (optimize) - -This step resolves four cost-optimization checks **in parallel**, one subagent per check: - -- `ff-active-but-unreferenced` -- `ff-local-eval-polling-interval` -- `ff-local-eval-in-edge-handlers` -- `ff-test-ci-gating` - -All four are grounded in PostHog's [feature flag cutting-costs guide](https://posthog.com/docs/feature-flags/cutting-costs) and [local-evaluation guide](https://posthog.com/docs/feature-flags/local-evaluation). The billed endpoint is `/flags` — references to the "decide endpoint" in older docs map to it. - -**Two of these checks (`ff-local-eval-polling-interval` and `ff-local-eval-in-edge-handlers`) only apply when server-side local evaluation is in use.** Step 1 recorded whether local eval was detected (server SDK initialized with `personal_api_key` / feature-flags secure API key, or a call to `getAllFlagsAndPayloads` / `getAllFlags`). If local eval was **not** detected, the two checks resolve with `status: "pass"` and `details: "skip: local evaluation not detected"` — do not dispatch their subagents at all. - -One check (`ff-active-but-unreferenced`) requires PostHog MCP access. If the MCP server is unavailable, auth fails, or any call errors after one retry: resolve with `suggestion`, `mcp_skipped: true`, and `details: "PostHog MCP unavailable — could not list active flags"`. Do not block the audit. - -{{> mcp-tool-calling}} - -## Status - -Emit before dispatching: - -``` -[STATUS] Auditing feature flag cost optimization -``` - -## Action — dispatch subagents in one message - -Make **one `Agent` tool call per check that actually runs** in a single message so they run concurrently. For each local-eval-gated check that is skipping, emit its `audit_resolve_checks` update directly (as a `pass` with skip details) instead of dispatching a subagent. Wait for all dispatched subagents to return, then continue to `4-report.md`. Do not run any other tools between dispatch and the next step. - -The bundled `cutting-costs.md` reference holds PostHog's authoritative cost-reduction guidance. It's typically at `.claude/skills/audit-feature-flags/references/cutting-costs.md`; if that path doesn't exist, discover it with `Glob` `**/skills/audit-feature-flags/references/cutting-costs.md`. Each subagent reads it once before judging. - -### Task A — `ff-active-but-unreferenced` - -`description`: `Audit ff-active-but-unreferenced` - -`prompt`: -``` -You are an audit subagent. Resolve exactly one rule and return: ff-active-but-unreferenced. - -This check requires PostHog MCP access. If the MCP server is unavailable, auth fails, or any call errors after one retry: resolve with `suggestion`, with `details` set to compact JSON `{"mcp_skipped": true, "reason": "PostHog MCP unavailable — could not list active flags"}`. Do not block the audit. - -Read this skill's bundled `cutting-costs.md` reference once (typically `.claude/skills/audit-feature-flags/references/cutting-costs.md`; otherwise discover with `Glob` `**/skills/audit-feature-flags/references/cutting-costs.md`). Focus on the "unused flags still incur charges" callout — active flags continue to evaluate (and bill) even with zero code references, because survey targeting and the `/flags` endpoint evaluate all active flags. The only way to stop charges is to disable, delete, or archive the flag in PostHog (removing it from code is not enough). - -Step 1 — list active flags from PostHog. Prefer `feature-flag-get-all` or the equivalent listing tool. If only `execute-sql` is available, fall back to: - -```sql -SELECT key -FROM feature_flags -WHERE active = true AND deleted = false -``` - -Step 2 — for each active flag key, grep the codebase for the literal key (case-sensitive). Count any reference in any source-tree file (a flag is referenced even if it appears only in a config file, a test fixture, or a comment). - -Rule: -- pass: every active flag has at least one reference in the codebase. -- suggestion: 1+ active flags have zero codebase references — they are still being evaluated (and billed) on every `/flags` request, especially via survey targeting. Recommend disabling, archiving, or deleting them in PostHog. - -Emit one `mcp__wizard-tools__audit_resolve_checks` call with a single update for id `ff-active-but-unreferenced`, with `file` left blank (this finding is project-wide, not tied to a single code site), and `details` as compact JSON: - -``` -{ - "active_flag_count": , - "unreferenced_active_flag_count": , - "unreferenced_keys": ["", ...], - "mcp_skipped": false -} -``` - -Return when the call completes. Do not write the audit report. -``` - -### Task B — `ff-local-eval-polling-interval` - -**Skip this task entirely if Step 1 did not detect local evaluation.** In that case, emit a direct `audit_resolve_checks` update for `ff-local-eval-polling-interval` with `status: "pass"` and `details: "skip: local evaluation not detected"`. - -`description`: `Audit ff-local-eval-polling-interval` - -`prompt`: -``` -You are an audit subagent. Resolve exactly one rule and return: ff-local-eval-polling-interval. - -Read this skill's bundled `cutting-costs.md` reference once (typically `.claude/skills/audit-feature-flags/references/cutting-costs.md`; otherwise discover with `Glob` `**/skills/audit-feature-flags/references/cutting-costs.md`). Focus on the "reducing local evaluation costs" section — by default, PostHog fetches flag definitions every 30 seconds. Each request is billed as 10 credits, so a constantly-running server makes `10 * 2 * 60 * 24 * 30 = 864,000` credits / month at the default. Increasing the polling interval (e.g. to 5 minutes) cuts that proportionally, at the cost of slower propagation of flag changes. - -Run **one** Grep: `featureFlagsPollingInterval|feature_flags_polling_interval|featureFlagsRequestTimeoutMs|feature_flag_request_timeout_ms`. - -Read each file that contains a server SDK init, once (locate via the Step 1 local-eval signals if needed: `personal_api_key` / `PostHog(`). For each init that uses local evaluation, determine whether `featureFlagsPollingInterval` (or the language-equivalent: `feature_flags_polling_interval`, `personal_api_key_request_timeout_seconds`, etc.) is set. - -Rule: -- pass: every local-eval init sets `featureFlagsPollingInterval` (or equivalent) to a non-default value, OR sets it explicitly to the default with an intentional comment. -- suggestion: polling interval is unset (defaulting to 30s) — at constant load that's ~864k `/flags` credits / month. Recommend setting a larger interval (e.g. 300_000 ms / 5 min) if real-time flag updates are not required. -- warning: polling interval is set to a value **smaller** than the 30s default (e.g. 10s) — increases cost without operational benefit. - -Emit one `mcp__wizard-tools__audit_resolve_checks` call with a single update for id `ff-local-eval-polling-interval`, including `file` (path:line of the local-eval init) and `details` as compact JSON: - -``` -{ - "polling_interval_ms": , - "uses_default": true | false, - "estimated_monthly_credits": , - "examples": [ - {"file": "", "issue": "unset-default | sub-default"} - ] -} -``` - -Return when the call completes. Do not write the audit report. -``` - -### Task C — `ff-local-eval-in-edge-handlers` - -**Skip this task entirely if Step 1 did not detect local evaluation.** In that case, emit a direct `audit_resolve_checks` update for `ff-local-eval-in-edge-handlers` with `status: "pass"` and `details: "skip: local evaluation not detected"`. - -`description`: `Audit ff-local-eval-in-edge-handlers` - -`prompt`: -``` -You are an audit subagent. Resolve exactly one rule and return: ff-local-eval-in-edge-handlers. - -Read this skill's bundled `cutting-costs.md` reference once (typically `.claude/skills/audit-feature-flags/references/cutting-costs.md`; otherwise discover with `Glob` `**/skills/audit-feature-flags/references/cutting-costs.md`). Focus on the edge/Lambda callout — local evaluation in an edge or Lambda environment initializes a PostHog instance on every invocation, which defeats the polling cache and inflates cost drastically. For these environments, use regular flag evaluation, or share flag definitions via an external cache (see local-evaluation/distributed-environments). - -Run **two** Greps in parallel: -- `posthog\.init\(|new PostHog\(|posthog\.Posthog\(|Posthog\(` — every PostHog init site. -- `runtime\s*=\s*['"]edge['"]|export\s+const\s+runtime|export\s+const\s+config\s*=\s*\{[^}]*runtime|lambda|exports\.handler|handler\s*:\s*async\s*\(|app/api/.*/route\.(ts|js)` — edge / Lambda handler signals. - -Read each file that contains a PostHog init, once. For each init, classify whether the file is an edge handler (`runtime = 'edge'`, `app/api/*/route.ts` on Next.js edge runtime, Vercel/Cloudflare edge, Lambda handler shape `exports.handler` / `handler: async (event) =>`, or paths under `lambda/` / `edge/` / `functions/`). For each edge/Lambda file, check whether the init is configured for **local evaluation** (presence of `personal_api_key` / feature-flags secure key, or calls to `getAllFlagsAndPayloads` / `getAllFlags`). - -Rule: -- pass: no PostHog init runs in an edge / Lambda handler, OR every edge/Lambda init is configured for remote (non-local) evaluation. -- error: a PostHog init in an edge / Lambda handler is configured for local evaluation — per-invocation init negates the polling cache and inflates cost. -- warning: a PostHog init in an edge / Lambda handler has ambiguous configuration (e.g. reuses a shared init module that does configure local evaluation, but only some call sites are edge-runtime). - -Emit one `mcp__wizard-tools__audit_resolve_checks` call with a single update for id `ff-local-eval-in-edge-handlers`, including `file` (path:line of the offending edge/Lambda init) and `details` as compact JSON: - -``` -{ - "edge_init_count": , - "edge_local_eval_count": , - "examples": [ - {"file": "", "issue": "local-eval-in-edge | ambiguous-shared-init"} - ] -} -``` - -Return when the call completes. Do not write the audit report. -``` - -### Task D — `ff-test-ci-gating` - -`description`: `Audit ff-test-ci-gating` - -`prompt`: -``` -You are an audit subagent. Resolve exactly one rule and return: ff-test-ci-gating. - -Read this skill's bundled `cutting-costs.md` reference once (typically `.claude/skills/audit-feature-flags/references/cutting-costs.md`; otherwise discover with `Glob` `**/skills/audit-feature-flags/references/cutting-costs.md`). Focus on the "configuring test and CI environments" section — test runners, CI pipelines, and staging environments often don't need real-time flag evaluation but silently rack up `/flags` requests on every init. The recommended pattern is to detect the test/CI environment (`process.env.NODE_ENV === 'test'`, `process.env.CI`, `BuildConfig.DEBUG`, etc.) and either skip init, disable flags (`advanced_disable_feature_flags: true`), or bootstrap deterministically. - -Run **three** Greps in parallel: -- `posthog\.init\(|new PostHog\(|posthog\.Posthog\(|Posthog\(` — every PostHog init site. -- `NODE_ENV.*test|process\.env\.CI|ProcessInfo\.processInfo\.environment\["CI"\]|BuildConfig\.DEBUG` — test/CI detection signals near init. -- `jest\.config|vitest\.config|playwright\.config|cypress\.config|\.test\.|\.spec\.|__tests__|tests/` (use `output_mode: "files_with_matches"`) — does the project have a test runner at all? - -If the third grep returns zero hits, resolve `pass` with `details: "skip: no test runner detected"` and return — this rule only applies to projects that actually run tests. - -Otherwise, read each file that contains a PostHog init, once. For each init, determine whether it is gated by a test/CI check: -- An `if (process.env.NODE_ENV !== 'test')` guard around the whole init call. -- An `advanced_disable_feature_flags: true` (or `preloadFeatureFlags: false`) conditional spread into the options when in test/CI. -- An early-return / `null` SDK shim in test mode. - -Rule: -- pass: every PostHog init is either gated against test/CI, or disables flags / preloading in test/CI, or the project's test runner setup makes the init unreachable in tests (e.g. a global setup file that monkey-patches PostHog). -- suggestion: init is unconditional but the project's tests do not appear to exercise it heavily (1–2 test files importing the init module). -- warning: init is unconditional and the project's test suite has 3+ files that load it — each test run silently incurs `/flags` requests. - -Emit one `mcp__wizard-tools__audit_resolve_checks` call with a single update for id `ff-test-ci-gating`, including `file` (path:line of the init that lacks gating) and `details` as compact JSON: - -``` -{ - "init_call_count": , - "test_gated_count": , - "test_runner_detected": true | false, - "examples": [ - {"file": "", "issue": "unguarded-in-tests"} - ] -} -``` - -Return when the call completes. Do not write the audit report. -``` - -## After all four checks are resolved - -Continue to **`4-report.md`**. diff --git a/context/skills/audit-feature-flags/references/4-report.md b/context/skills/audit-feature-flags/references/4-report.md deleted file mode 100644 index b1ad9122..00000000 --- a/context/skills/audit-feature-flags/references/4-report.md +++ /dev/null @@ -1,125 +0,0 @@ ---- -next_step: null ---- - -# Step 4 — Generate the audit report - -The audit report is rendered **directly from `.posthog-audit-checks.json`** — that file is the source of truth. Every check the wizard seeded for this skill ends up in the report, even passes; nothing is invented. - -## Status - -Emit: - -``` -[STATUS] Writing feature flag audit report -``` - -## Action - -`Read` the ledger once, then transform every entry into the report below. Use `area`, `label`, `status`, `file`, and `details` from each entry verbatim where the report calls for them. - -`Write` `posthog-audit-feature-flags-report.md` at the project root with the structure shown below. After the report is written, delete `.posthog-audit-checks.json`. - -The report has four sections in this order: - -1. **Summary** — one-paragraph overview, severity counts, and a problematic-items table. -2. **Recommended actions** — prioritized fixes and optimizations with `file:line` where applicable. -3. **Full audit** — every check the wizard ran, grouped by `area`, including passes. -4. **About this audit** — short closing block explaining what this audit covered. - -For the Full audit section, group rows by each distinct `area` value in the ledger, preserving first-seen area order from the JSON. This skill produces two areas: **Feature Flags** (fix) and **Feature Flags — Optimize** (cost). Render whatever areas the ledger actually contains. - -For each area, write a one-paragraph framing immediately under the area heading, then the table. - -## Report template - - -# PostHog Feature Flags Audit Report - -## Summary - -[1–2 sentence overview: runtimes covered (client/server/both), whether local evaluation is in use, and which lens — fix, optimize, or both — surfaced issues.] - -**Counts** - -- **Errors**: [N] (must fix) -- **Warnings**: [N] (should fix) -- **Suggestions**: [N] (nice to have / cost savings) -- **Passes**: [N] - -**Problematic items** _(only `error`, `warning`, `suggestion` — no passes)_ - -| Severity | Area | Check | File | Details | -|----------|------|-------|------|---------| -| `error` | Feature Flags | [label] | [file:line] | [details] | - -If there are no problematic items, write `_No issues found — your feature flag setup looks healthy._` instead of the table. - -## Recommended actions - -Numbered list, ordered by severity (errors → warnings → suggestions), then by area within a severity (Feature Flags → Feature Flags — Optimize). Each item is **three sentences**, in this order: - -1. **What's wrong** — the finding, written as a one-sentence diagnosis derived from `details`. -2. **Why it matters** — one sentence on the correctness or cost consequence. For fix-side checks: which user-facing artifact (flicker, wrong variant, runtime error) this finding causes. For optimize-side checks: the billing or volume impact, quoting the ratio or count from `details`. -3. **How to fix** — one short imperative sentence pointing at `file:line` (or "no specific code site — adjust SDK config" for MCP-only findings) and the concrete change. End with a docs link. - -Format: - -1. **[Area] · [label]** — [what's wrong]. _Why it matters:_ [why-it-matters]. _Fix:_ [how-to-fix at `file:line`]. See [docs]([area docs url]). - -Suggested docs URLs: -- `ff-bootstrap-when-known-set` → https://posthog.com/docs/feature-flags/bootstrapping -- `ff-await-readiness`, `ff-default-values` → https://posthog.com/docs/feature-flags/best-practices -- `ff-active-but-unreferenced` → https://posthog.com/docs/feature-flags/cutting-costs -- `ff-local-eval-polling-interval`, `ff-local-eval-in-edge-handlers` → https://posthog.com/docs/feature-flags/local-evaluation -- `ff-test-ci-gating` → https://posthog.com/docs/feature-flags/cutting-costs - -If there are no actions, write `_Nothing to fix._`. - -## Full audit - -### Feature Flags - -This area covers correctness of feature flag usage: that flags are bootstrapped when an initial set is known at app start, that flag-eval calls await readiness (so they don't fire before flags load and misread `undefined`), and that every flag-eval has a sensible default fallback for loading-window, network-failure, and quota-limited scenarios. - -| Check | Status | File | Details | -|-------|--------|------|---------| -| [label] | [status] | [file] | [details] | - -### Feature Flags — Optimize - -This area covers cost-side `/flags` health: unreferenced-but-active flags that continue to evaluate (and bill) on every request, the local-evaluation polling interval (defaults to 30s = ~864k credits/month at constant load), local evaluation running inside edge/Lambda handlers (which negates the polling cache), and test/CI gating to prevent silent `/flags` accrual in test runs. Optimize checks that target server-side local evaluation are skipped as `pass` when local evaluation is not detected. MCP-backed checks resolve as `suggestion` with `mcp_skipped: true` in `details` when PostHog MCP is unavailable. - -| Check | Status | File | Details | -|-------|--------|------|---------| -| [label] | [status] | [file] | [details] | - -[Repeat the heading + paragraph + table for each area in ledger order, in case future versions of this skill add new areas.] - -### Assumptions and blind spots - -Under each area's table above, render a `### Assumptions and blind spots` subsection per the investigation standards in `posthog-best-practices/references/investigation-standards.md` (standard 3). Answer the four questions in plain prose, ≤4 sentences total: -- Which code paths or files this area did NOT check that could change the findings. -- Which runtime assumptions are unproven by the static code (mount order, async timing, route gating). -- Alternative explanations for the patterns the checks flagged. -- What you would verify in the live PostHog project (event volumes, property fill rates, dashboard usage) to confirm or refute the most important findings. - -When an area produced only `pass` rows, write `_No findings to qualify; the standard checks for this area passed cleanly._` and skip the four-question rundown. - -## About this audit - -This audit ran the PostHog `audit-feature-flags` skill — a focused, read-only check of feature flag health across two lenses: **fix** (correctness) and **optimize** (cost). Fix checks scan the project source; optimize checks additionally query the PostHog project via MCP in read-only mode (and gracefully skip when MCP is unavailable). The billed endpoint is `/flags` (the renamed `/decide`). - -- `error` items break correctness now (flicker, wrong variant, runtime error). Fix first. -- `warning` items work today but cause subtle bugs or noticeably elevated cost. Fix when convenient. -- `suggestion` items are best-practice improvements or cost-savings opportunities with measurable upside. - -Re-run `posthog-wizard audit-feature-flags` after applying fixes to refresh the ledger. - - - -After the report is written, emit a final line so the wizard can surface the path to the user: - -``` -Created audit report: -``` diff --git a/context/skills/audit-feature-flags/references/checks.md b/context/skills/audit-feature-flags/references/checks.md new file mode 100644 index 00000000..a22515e9 --- /dev/null +++ b/context/skills/audit-feature-flags/references/checks.md @@ -0,0 +1,196 @@ +# Feature Flags Doctor — Checks + +Read this file when Phase 1 begins — not before. The `prompt-ff-*.md` files referenced below are subagent prompts: each dispatched subagent reads exactly its own, and the main loop never reads any of them. + +Two groups. **Static checks** read the source tree only and run first, as parallel subagents. **Live checks** run second, from the main loop (no subagents): they probe the project's real `/flags` endpoint and read the flag roster via MCP. Severity values are **fixed** — do not adjust them. Resolve every check via `mcp__wizard-tools__audit_resolve_checks` (skip ids missing from the ledger); append per-flag findings via `mcp__wizard-tools__audit_add_checks` exactly as each sweep section specifies. + +Every check is independent and required. A failure in one does not block the others. Do not invent checks beyond the ones listed. + +--- + +## Part 1 — Static checks (parallel subagents) + +Emit `[STATUS] Auditing feature flag correctness`, then make **six `Agent` tool calls in a single message** for the correctness checks (Tasks A–F). When all six return, emit `[STATUS] Auditing feature flag cost optimization` and dispatch the cost checks (Tasks G–I) the same way — one `Agent` call per check that actually runs, in a single message. Tasks G and H are gated on the pre-flight local-evaluation signal; for a gated task that is skipping, emit its `audit_resolve_checks` update directly (`status: "pass"`, `details: "skip: local evaluation not detected"`) instead of dispatching a subagent. + +### Dispatch table + +| Task | Check id | Prompt file | Gating | +| --- | --- | --- | --- | +| A | `ff-bootstrap-when-known-set` | `prompt-ff-bootstrap-when-known-set.md` | — | +| B | `ff-await-readiness` | `prompt-ff-await-readiness.md` | — | +| C | `ff-default-values` | `prompt-ff-default-values.md` | — | +| D | `ff-bootstrap-distinct-id-mismatch` | `prompt-ff-bootstrap-distinct-id-mismatch.md` | — | +| E | `ff-identified-only-pre-auth-targeting` | `prompt-ff-identified-only-pre-auth-targeting.md` | — | +| F | `ff-eval-before-identify` | `prompt-ff-eval-before-identify.md` | — | +| G | `ff-local-eval-polling-interval` | `prompt-ff-local-eval-polling-interval.md` | skip unless local evaluation detected | +| H | `ff-local-eval-in-edge-handlers` | `prompt-ff-local-eval-in-edge-handlers.md` | skip unless local evaluation detected | +| I | `ff-test-ci-gating` | `prompt-ff-test-ci-gating.md` | — | + +### Dispatch shape — identical for every task + +Each `Agent` call sets: + +- `description`: `Audit ` +- `prompt` — this template verbatim, substituting `` and `` from the table: + +```` +You are an audit subagent. Resolve exactly one rule and return: . + +Read this skill's bundled `` reference once (typically `.claude/skills/audit-feature-flags/references/`; otherwise discover it with `Glob` `**/skills/audit-feature-flags/references/`) and follow its instructions exactly. Emit the single `mcp__wizard-tools__audit_resolve_checks` call it specifies, then return. Do not write the audit report. +```` + +Do not paste a prompt file's contents into the dispatch prompt — the subagent loads its own. The prompt files direct each subagent to the bundled doc (`best-practices.md`, `bootstrapping.md`, or `cutting-costs.md`) that holds PostHog's authoritative guidance for its rule. + +--- + +## Part 2 — Live checks (main loop, sequential) + +These run after the static fan-out returns. They need two inputs; establish both up front: + +**The project token and host.** Prefer the values the wizard passed in the run context (project API key `phc_…` and API host). Otherwise: `mcp__wizard-tools__check_env_keys` to learn which env keys exist, then Grep/Read the init site to find how the token and `api_host` are wired (the token is a public client value; personal API keys are never used here). Record BOTH hosts when they differ: the **app path** (what the SDK is configured to use — often a first-party proxy like `/ingest` on the app's own domain) and the **direct cloud host** (`https://us.i.posthog.com` or `https://eu.i.posthog.com` by region). + +**The flag roster.** List the project's flags via `feature-flag-get-all` (or the equivalent MCP listing tool). Fallback if only SQL is available: + +```sql +SELECT key, active, filters +FROM feature_flags +WHERE deleted = false +``` + +If the roster call fails with a permissions error, emit `[ABORT] Insufficient permissions`. If the MCP server is simply unavailable, resolve the roster-dependent comparisons (`ff-flags-delivered`, `ff-unknown-flags`, `ff-stale-rolled-out`, `ff-active-but-unreferenced`) as `suggestion` with `details: {"mcp_skipped": true, "reason": "PostHog MCP unavailable"}`. + +**MCP loss degrades ONLY the roster comparisons — nothing else.** Every check below that runs on probes or greps MUST still execute in full when MCP is down: Check J (`ff-key-authenticates` — curl only), Check K (`ff-flags-endpoint` — curl only), Check M's code-key collection (grep only; record the keys in `details` even when the roster comparison is skipped), and Check N's code signal (`ff-evaluated-not-reported` — grep only; a `send_event: false` suppression is a full-strength warning with or without the data signal, and it still sets `gates_cleanup: true`). Resolve every check individually; never batch-skip the live phase because one dependency failed. + +### The probe (used by Checks J, K, and L) + +Emit `[STATUS] Probing /flags delivery`. One plain curl per target host — **the User-Agent matters** (tenet 4): PostHog's server filters clients whose UA looks automated (contains `HeadlessChrome`, `bot`, crawler names, etc.) and returns `{"errorsWhileComputingFlags": false, "flags": {}}` with HTTP 200 for them. That is expected product behavior, and it means a probe with a default curl-adjacent or headless UA manufactures a false failure. Always send a realistic browser UA: + +``` +curl -s -X POST "/flags/?v=2" \ + -H "Content-Type: application/json" \ + -A "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/126.0.0.0 Safari/537.36" \ + -d '{"api_key": "", "distinct_id": "ff-doctor-probe"}' +``` + +Run it once against the direct cloud host and — when the app is configured with a different `api_host` — once against the app path (e.g. `https://app.example.com/ingest/flags/?v=2`). Keep both raw responses in memory for the checks below. Do not retry more than once per host. + +### Check J — `ff-key-authenticates` + +What it verifies: the token the app actually ships **authenticates**. Static analysis can see an env var referenced; it cannot know the value works — or that the env file is even populated (an empty `.env` fails silently: the SDK inits, nothing ever arrives). + +Classify the direct-host probe: +- HTTP 200 with a JSON body (flags present or legitimately empty) → the key authenticates. +- HTTP 401 / 403, or a body with an authentication error → the key is wrong or revoked. +- No token found anywhere in env or code → the key is not wired at all. + +Rule: +- pass: 200 with valid JSON. +- error: 401/403/auth error — the app cannot receive flags with this key. +- error: no token wired — the SDK initializes with nothing; flags silently never load. (`file`: the init site expecting the env var.) +- error: a **personal API key** (`phx_…` pattern, or an env var named like `POSTHOG_PERSONAL_API_KEY`) is referenced in client-side code or a client bundle path — personal keys grant account access and must never ship to browsers; flags client-side use the public project key. (`file`: the offending reference. Report the variable name only, never the value.) +- suggestion: probe could not run (no network / no token discoverable AND wizard context absent) — `details` explains. + +`details` (compact JSON): `{"status_code": , "host": "", "token_source": "wizard-context | env | code | missing"}`. Never include the token value. + +### Check K — `ff-flags-endpoint` + +What it verifies: the **path the app actually uses** serves flags. Projects using a first-party reverse proxy (`api_host: "/ingest"` etc.) can break the flags route in the proxy rewrite while everything else looks fine — a class of silent failure the static audit cannot see. + +Only meaningful when the app path differs from the direct host; if the app talks to PostHog Cloud directly, resolve `pass` with `details: "app uses the direct PostHog host"`. + +Compare the two probe responses: +- Both 200 with the same flag keys → pass. +- Direct host healthy but the app path errors (non-200, HTML error page, timeout) → **error**: the proxy route is broken; every client behind it silently gets no flags. +- Direct host healthy but the app path returns 200 with a *different* (e.g. empty) flag set → **warning**: something between the app and PostHog is altering the response. + +`details`: `{"direct_status": , "proxy_status": , "direct_flag_count": , "proxy_flag_count": , "proxy_host": ""}`. + +Emit `[STATUS] Cross-checking delivered flags against definitions` before the next check. + +### Check L — `ff-flags-delivered` (sweep) + +What it verifies: every **active** flag in the roster actually appears in the probe response, and what each evaluates to. `/flags?v=2` returns a per-flag `reason` (e.g. `condition_match`, `out_of_rollout_bound`) — surface it: it converts "my flag isn't working" from a mystery into a stated cause. + +Compare roster (active, non-deleted flags) against the app-path probe response (fall back to the direct response if there is no proxy): + +- Every active flag present in the response → resolve `ff-flags-delivered` as `pass` with `details: {"active_flag_count": , "delivered_count": }`. +- One or more active flags missing from the response, or present with a surprising evaluation → resolve `ff-flags-delivered` as `warning` with the counts, AND append one row per affected flag via a single `audit_add_checks` call: + - `id`: `delivered-` (kebab-case; append `-2` on collision — a duplicate id rejects the whole batch; never call with an empty array). + - `area`: `Feature Flags — Delivery`. + - `label`: ` not delivered` (≤40 chars, no trailing period). + - `status`: `warning`, `file`: omit. + - `details`: one line — what the roster says vs what the probe returned, including the `reason` code when present. + +Report copy note (teaching callout, NOT a finding): if the operator's own verification method is an automated browser (Playwright/Cypress/headless), remind them in the report's "Notes on expected behavior" section that such clients intentionally receive zero flags — with the docs link — because that is the single most common false alarm when people test flags. + +### Check M — `ff-unknown-flags` (sweep) + +Emit `[STATUS] Checking flag keys referenced in code exist in PostHog`. + +What it verifies: the **reverse direction** — every flag key referenced in code exists in the roster. A typo'd or deleted key returns `undefined` on every evaluation, forever, with no error anywhere. Check P covers tenant→code (unreferenced flags); this is code→tenant, and nothing else covers it. + +Collect the set of flag keys used in code: Grep flag-eval call sites (`getFeatureFlag\(|isFeatureEnabled\(|useFeatureFlag\(|getFeatureFlagPayload\(`) and extract the first string-literal argument of each call. Keys built dynamically (template strings, variables) are recorded as `dynamic` and excluded from the comparison (note the count in `details`). + +Compare against the roster (all non-deleted flags, active or not): +- Every literal key exists in the roster → resolve `ff-unknown-flags` as `pass` with `details: {"code_key_count": , "dynamic_key_count": }`. +- One or more keys missing → resolve `ff-unknown-flags` as `error` with the counts, AND append one row per ghost key via a single `audit_add_checks` call: + - `id`: `ghost-`, `area`: `Feature Flags — Delivery`, `label`: ` not found in PostHog` (≤40 chars), `status`: `error`, `file`: the `path:line` of the call site, `details`: one line naming the nearest-matching roster key if one exists (likely typo) or `no similar flag — deleted or never created`. + +### Check N — `ff-evaluated-not-reported` + +Emit `[STATUS] Verifying evaluation events are reported`. + +What it verifies: flags being **evaluated** and evaluations being **reported** are different things. SDKs report evaluations via `$feature_flag_called` events; PostHog uses those events for experiment exposure AND for flag staleness ("not evaluated in 30+ days"). A project that evaluates flags but suppresses these events breaks its experiments silently — and makes every used flag look stale, which is why this check gates cleanup (tenet 2). + +Two signals, either sufficient for a finding: + +1. **Code signal:** Grep for `send_feature_flag_events|sendFeatureFlagEvents|sendFeatureFlagEvent|advanced_disable_feature_flags|send_event\s*:\s*false|sendEvent\s*:\s*false` and read each hit. Two suppression forms count: config-level (an init/config option that disables flag-called events) and per-call (posthog-js `isFeatureEnabled('key', { send_event: false })` / `getFeatureFlag(..., { send_event: false })`). A suppression in production paths (not test/CI-gated blocks — those are the correct pattern from Task I) is a finding; per-call suppressions matter per flag — a single suppressed flag is enough to make THAT flag look stale. +2. **Data signal (when MCP query access is available):** count recent `$feature_flag_called` events: + +```sql +SELECT count() AS calls +FROM events +WHERE event = '$feature_flag_called' + AND timestamp > now() - INTERVAL 7 DAY +``` + +Rule: +- pass: no production-path suppression in code AND (query unavailable OR calls > 0 OR the project has no production traffic yet — do not fail a brand-new project on zero events alone; record `details: "no traffic baseline"`). +- warning: code suppresses `$feature_flag_called` in production paths, OR the project has flag call sites and recent traffic but zero `$feature_flag_called` events in 7 days. +- error: suppression is unconditional AND the roster shows flags used in experiments (roster `filters`/experiment linkage) — experiment exposure is silently broken. + +`details` (compact JSON): `{"suppression_sites": [{"file": ""}], "calls_7d": , "gates_cleanup": true | false}` — set `gates_cleanup: true` whenever status is warning/error; Phase 2 reads this to withhold tenant-side cleanup options. + +### Check O — `ff-stale-rolled-out` (sweep) + +What it verifies: flags at **100% rollout with no conditions** that are still gated in code — the check is equivalent to a hardcoded value: dead branches, needless evaluations, cleanup candidates. (The tenant-side twin, zero-reference active flags, is Check P below — `ff-active-but-unreferenced`.) + +From the roster, select active flags whose filters release to 100% with no property/cohort conditions and no experiment linkage. Intersect with the code-key set from Check M (flags that ARE referenced). + +- None → resolve `ff-stale-rolled-out` as `pass`. +- One or more → resolve as `suggestion` with counts, AND append one row per flag via a single `audit_add_checks` call: `id`: `stale-`, `area`: `Feature Flags — Optimize`, `label`: ` at 100% but still gated` (≤40 chars), `status`: `suggestion`, `file`: a representative call site, `details`: one line — rollout state + reference count + "safe order: remove the gate, deploy, then disable in PostHog". + +**Interlock note:** when `ff-evaluated-not-reported` has `gates_cleanup: true`, these rows still appear (they're real observations) but their fixes are withheld in Phase 2 and the report explains why. + +### Check P — `ff-active-but-unreferenced` + +What it verifies: the tenant→code direction of drift — **active** flags with zero references anywhere in the source tree. Per the cutting-costs guidance: active flags continue to evaluate (and bill) even with zero code references, because survey targeting and the `/flags` endpoint evaluate all active flags. The only way to stop the charges is to disable, archive, or delete the flag in PostHog — removing it from code is not enough. + +Using the roster (active flags) and the code-key set from Check M: for each active flag key with zero literal references, grep once more for the literal key across the whole tree (any file counts — config, test fixture, comment) to rule out non-eval references before flagging. + +Rule: +- pass: every active flag has at least one reference in the codebase. +- suggestion: 1+ active flags have zero references — still evaluated (and billed) on every `/flags` request. Recommend disabling, archiving, or deleting them in PostHog. + +Resolve `ff-active-but-unreferenced` with `file` omitted (project-wide) and `details` as compact JSON: + +``` +{ + "active_flag_count": , + "unreferenced_active_flag_count": , + "unreferenced_keys": ["", ...], + "mcp_skipped": false +} +``` + +**Interlock note:** archive fixes for these flags are the ones most directly gated by `ff-evaluated-not-reported` — "zero code references" and "zero evaluation events" are only trustworthy together when evaluation reporting is verified working. diff --git a/context/skills/audit-feature-flags/references/prompt-ff-await-readiness.md b/context/skills/audit-feature-flags/references/prompt-ff-await-readiness.md new file mode 100644 index 00000000..0f20018d --- /dev/null +++ b/context/skills/audit-feature-flags/references/prompt-ff-await-readiness.md @@ -0,0 +1,35 @@ +--- +description: Subagent prompt for ff-await-readiness — read only by its dispatched subagent, never by the main loop +--- + +You are an audit subagent. Resolve exactly one rule and return: ff-await-readiness. + +Read this skill's bundled `best-practices.md` reference once (typically `.claude/skills/audit-feature-flags/references/best-practices.md`; otherwise discover with `Glob` `**/skills/audit-feature-flags/references/best-practices.md`). Focus on the readiness / "have the value before you need it" section — client-side flag evaluation is async, so any flag-eval before `onFeatureFlags` fires (or before the `loaded` callback runs, or before `bootstrap.featureFlags` is set) returns `undefined`, which is **not** `false`. Misreading the loading gap is one of the most common flag bugs. + +Run **three** Greps in parallel: +- `getFeatureFlag\(|isFeatureEnabled\(|useFeatureFlag\(|getFeatureFlagPayload\(` — every flag-eval call site. +- `onFeatureFlags\(|posthog\.onFeatureFlags\(` — readiness subscribers. +- `bootstrap\s*:|loaded\s*:|loaded\s*\(` — bootstrap config and `loaded` callbacks on init. + +Read each file that contains a flag-eval hit, once. For each flag-eval call, determine whether it is gated against the loading window — i.e. it happens after `onFeatureFlags` fires, inside / after a `loaded` callback, after a bootstrap was provided, or behind a readiness guard the project defines itself. + +Rule: +- pass: every flag-eval call site is either bootstrapped, behind `onFeatureFlags` / `loaded` gating, or inside a code path that only runs post-init (e.g. a click handler). +- warning: one or more flag-eval calls run in a render-on-mount path (React render body, `useEffect` with empty deps, Vue `onMounted`) without bootstrap or readiness gating — race-condition risk. +- error: a flag-eval call's return value is compared with `===` / `!==` to a non-undefined value in a path that can run before flags load, and the codebase has no bootstrap and no readiness subscribe — guaranteed undefined-handling bug. + +Emit one `mcp__wizard-tools__audit_resolve_checks` call with a single update for id `ff-await-readiness`, including `file` (path:line of the most representative offending flag-eval call) and `details` as compact JSON: + +``` +{ + "flag_eval_call_count": , + "ungated_call_count": , + "bootstrap_present": true | false, + "readiness_subscriber_present": true | false, + "examples": [ + {"file": "", "issue": "race-on-mount | undefined-misread"} + ] +} +``` + +Return when the call completes. Do not write the audit report. diff --git a/context/skills/audit-feature-flags/references/prompt-ff-bootstrap-distinct-id-mismatch.md b/context/skills/audit-feature-flags/references/prompt-ff-bootstrap-distinct-id-mismatch.md new file mode 100644 index 00000000..0a3aca0a --- /dev/null +++ b/context/skills/audit-feature-flags/references/prompt-ff-bootstrap-distinct-id-mismatch.md @@ -0,0 +1,38 @@ +--- +description: Subagent prompt for ff-bootstrap-distinct-id-mismatch — read only by its dispatched subagent, never by the main loop +--- + +You are an audit subagent. Resolve exactly one rule and return: ff-bootstrap-distinct-id-mismatch. + +Read this skill's bundled `bootstrapping.md` reference once (typically `.claude/skills/audit-feature-flags/references/bootstrapping.md`; otherwise discover with `Glob` `**/skills/audit-feature-flags/references/bootstrapping.md`). + +Background: `bootstrap.distinctID` (or `bootstrap: { distinctID: ... }`) lets the host application seed the SDK's distinct_id at init time — usually for SSR/SSG scenarios where the server already knows the user. But if the value passed doesn't match either the user's eventual stable id (after `identify()`) or the SDK's natural anonymous id, it overrides the identity chain in ways that break later merges. Two failure modes: +1. `distinctID` set to a per-request random / session UUID — the SDK considers itself "already identified" with that UUID; the next `identify(realUserId)` is blocked from merging anonymous activity. +2. `distinctID` set to a known user id but the project ALSO calls `identify(differentId)` shortly after — the two ids race; whichever loses creates an orphan profile. + +Run **two** Greps in parallel: +- `bootstrap[\s\S]{0,40}distinctID|bootstrap[\s\S]{0,40}distinct_id|distinctID\s*:` — bootstrap-with-distinctID sites. +- `posthog\.identify\(` — every identify call (so the subagent can cross-reference). + +Read each file that contains a bootstrap.distinctID hit, once. For each site, determine: +- What value is being passed (literal, variable, request-scoped, randomly generated)? +- Is the same value later passed to `posthog.identify()`? If yes, that's the safe pattern (matching SSR hydration). +- Is the value request-scoped / per-render (e.g. `crypto.randomUUID()`, `Math.random()`, a Next.js per-request id)? If yes, this is the failure mode. + +Rule: +- pass: no `bootstrap.distinctID` usage detected, OR the bootstrapped value is stable across requests and matches the value passed to a later identify() call. +- warning: `bootstrap.distinctID` is set to a value that appears request-scoped, randomly generated, or otherwise volatile — the next identify() call will be blocked from merging anonymous activity. +- error: `bootstrap.distinctID` is set to one value and `posthog.identify()` is called immediately after with a DIFFERENT value on the same code path — orphan profile guaranteed. + +Emit one `mcp__wizard-tools__audit_resolve_checks` call with a single update for id `ff-bootstrap-distinct-id-mismatch`, including `file` (path:line of the bootstrap site) and `details` as compact JSON: + +``` +{ + "bootstrap_distinct_id_site_count": , + "examples": [ + {"file": "", "issue": "volatile-bootstrap-id | bootstrap-identify-mismatch | safe-ssr-hydration"} + ] +} +``` + +Return when the call completes. Do not write the audit report. diff --git a/context/skills/audit-feature-flags/references/prompt-ff-bootstrap-when-known-set.md b/context/skills/audit-feature-flags/references/prompt-ff-bootstrap-when-known-set.md new file mode 100644 index 00000000..a0e0f0a4 --- /dev/null +++ b/context/skills/audit-feature-flags/references/prompt-ff-bootstrap-when-known-set.md @@ -0,0 +1,39 @@ +--- +description: Subagent prompt for ff-bootstrap-when-known-set — read only by its dispatched subagent, never by the main loop +--- + +You are an audit subagent. Resolve exactly one rule and return: ff-bootstrap-when-known-set. + +Read this skill's bundled `best-practices.md` reference once (typically `.claude/skills/audit-feature-flags/references/best-practices.md`; otherwise discover with `Glob` `**/skills/audit-feature-flags/references/best-practices.md`). Focus on the bootstrapping guidance — when an initial flag set is already known at app start (e.g. computed server-side, persisted in a cookie, or passed through SSR props), client-side `posthog.init` should set `bootstrap.featureFlags` so the first render has the right values without a `/flags` round trip. + +Run **two** Greps in parallel: +- `posthog\.init\(|new PostHog\(|posthog\.Posthog\(|Posthog\(` — every PostHog init site. +- `getFeatureFlag\(|isFeatureEnabled\(|useFeatureFlag\(` — every flag-eval call site. + +Read each file that contains an init hit, once. For each init, inspect the options object: is `bootstrap.featureFlags` (or `bootstrap: { featureFlags: ... }`) provided? + +Then check whether the codebase has a known initial flag set referenced **before init returns** — common signals: +- SSR / server-rendered props that pass flag values into a `` / init call. +- A `cookies` / `headers` read that yields flag values, used near init. +- An explicit constant or map of flag keys imported into the init module. +- Flag-eval call sites running synchronously inside the same render path that mounts the provider. + +Rule: +- pass: bootstrap is set when a known initial flag set exists, OR no known initial set is referenced before init (nothing to bootstrap with). +- warning: a known initial flag set is referenced before init returns but `bootstrap.featureFlags` is not set on init — early flag evals will return `undefined` and cause flicker. +- suggestion: init has neither bootstrap nor any `onFeatureFlags` / `loaded` callback gating early evals — recommend either bootstrap (preferred) or readiness gating. + +Emit one `mcp__wizard-tools__audit_resolve_checks` call with a single update for id `ff-bootstrap-when-known-set`, including `file` (path:line of the init that lacks bootstrap) and `details` as compact JSON: + +``` +{ + "init_call_count": , + "init_with_bootstrap_count": , + "known_initial_set_detected": true | false, + "examples": [ + {"file": "", "issue": "missing-bootstrap | no-readiness-gate"} + ] +} +``` + +Return when the call completes. Do not write the audit report. diff --git a/context/skills/audit-feature-flags/references/prompt-ff-default-values.md b/context/skills/audit-feature-flags/references/prompt-ff-default-values.md new file mode 100644 index 00000000..459c75e8 --- /dev/null +++ b/context/skills/audit-feature-flags/references/prompt-ff-default-values.md @@ -0,0 +1,34 @@ +--- +description: Subagent prompt for ff-default-values — read only by its dispatched subagent, never by the main loop +--- + +You are an audit subagent. Resolve exactly one rule and return: ff-default-values. + +Read this skill's bundled `best-practices.md` reference once (typically `.claude/skills/audit-feature-flags/references/best-practices.md`; otherwise discover with `Glob` `**/skills/audit-feature-flags/references/best-practices.md`). Focus on the "undefined is not false" and per-flag default guidance — `getFeatureFlag('key')` returns `undefined` during the loading window and may also return `undefined` when PostHog is unreachable or quota-limited. A per-flag default (via `?? 'control'`, a wrapper helper, or the SDK's `default_value`/`defaultValue` option when supported) controls what users see during these windows. + +Run **one** Grep: `getFeatureFlag\(|isFeatureEnabled\(|useFeatureFlag\(` — every flag-eval call site. + +Read each file that contains a hit, once. For each flag-eval call, classify whether the result is consumed with a default-value fallback: +- **explicit `??` / `||` fallback** on the call expression — fine. +- **wrapped in a helper** that supplies a default (e.g. `function useBetaFeature() { return posthog.isFeatureEnabled('beta') ?? false }`) — fine. +- **explicit `=== 'variant'` / `!== 'variant'` comparison** treated as the default-handling — fine *only if* the surrounding code path can tolerate `undefined` (i.e. the variant branch is the opt-in and the fallthrough is safe). +- **bare consumption** — the call result feeds into a conditional, prop, or render without a default — flag. + +Rule: +- pass: every flag-eval call site either has a per-flag default fallback or is consumed via a safe variant comparison. +- suggestion: 1–2 bare flag-eval call sites — low risk, recommend adding `?? `. +- warning: 3+ bare flag-eval call sites, OR any bare call in a code path the user always hits (top-level render, app shell). + +Emit one `mcp__wizard-tools__audit_resolve_checks` call with a single update for id `ff-default-values`, including `file` (path:line of the most representative bare call) and `details` as compact JSON: + +``` +{ + "flag_eval_call_count": , + "bare_consumption_count": , + "examples": [ + {"file": "", "issue": "no-default-fallback"} + ] +} +``` + +Return when the call completes. Do not write the audit report. diff --git a/context/skills/audit-feature-flags/references/prompt-ff-eval-before-identify.md b/context/skills/audit-feature-flags/references/prompt-ff-eval-before-identify.md new file mode 100644 index 00000000..527c511d --- /dev/null +++ b/context/skills/audit-feature-flags/references/prompt-ff-eval-before-identify.md @@ -0,0 +1,35 @@ +--- +description: Subagent prompt for ff-eval-before-identify — read only by its dispatched subagent, never by the main loop +--- + +You are an audit subagent. Resolve exactly one rule and return: ff-eval-before-identify. + +Background: client-side flag evaluation resolves against the CURRENT distinct_id. A flag evaluated before `posthog.identify()` runs resolves against the anonymous id — targeting rules based on person properties or cohorts don't match, so the user gets one value pre-identify and potentially a different value after identify triggers a flag reload. Symptoms: UI flicker on login, users "randomly" switching variants, person-targeted flags never firing on first render. + +Run **two** Greps in parallel: +- `getFeatureFlag\(|isFeatureEnabled\(|useFeatureFlag\(|getFeatureFlagPayload\(` — where captures and flag evals happen. +- `posthog\.identify\(` — every identify call. + +Read each file that contains hits from either grep, once. Compare the timing/ordering of `identify()` against the surrounding flag-eval calls: +- flag-eval in a code path that runs on initial mount of an authenticated area, where `identify()` is called LATER in the same flow (after a session fetch, inside a login callback, in a child effect) — the eval races identify. +- flag-eval inside auth/bootstrap code that runs strictly after `identify()` resolves — safe. +- flag-eval on genuinely anonymous surfaces (no identify in the flow) — safe by definition; not this rule's concern. + +Rule: +- pass: no flag-eval races an identify() in the same flow, OR the project re-evaluates after identify (an `onFeatureFlags` subscriber re-renders, or an explicit `reloadFeatureFlags()` follows identify). +- suggestion: 1–2 racing sites with a re-evaluation path present but indirect — recommend making the ordering explicit. +- warning: any racing site with NO re-evaluation after identify — person-targeted flags will be evaluated against the anonymous id and stay wrong until the next natural reload. + +Emit one `mcp__wizard-tools__audit_resolve_checks` call with a single update for id `ff-eval-before-identify`, including `file` (path:line of the most representative racing eval) and `details` as compact JSON: + +``` +{ + "racing_site_count": , + "reeval_after_identify": true | false, + "examples": [ + {"file": "", "issue": "eval-races-identify"} + ] +} +``` + +Return when the call completes. Do not write the audit report. diff --git a/context/skills/audit-feature-flags/references/prompt-ff-identified-only-pre-auth-targeting.md b/context/skills/audit-feature-flags/references/prompt-ff-identified-only-pre-auth-targeting.md new file mode 100644 index 00000000..a13da87e --- /dev/null +++ b/context/skills/audit-feature-flags/references/prompt-ff-identified-only-pre-auth-targeting.md @@ -0,0 +1,41 @@ +--- +description: Subagent prompt for ff-identified-only-pre-auth-targeting — read only by its dispatched subagent, never by the main loop +--- + +You are an audit subagent. Resolve exactly one rule and return: ff-identified-only-pre-auth-targeting. + +Read this skill's bundled `best-practices.md` reference once (typically `.claude/skills/audit-feature-flags/references/best-practices.md`; otherwise discover with `Glob` `**/skills/audit-feature-flags/references/best-practices.md`). + +Background: when `person_profiles: 'identified_only'` is set (the recommended default for most B2B SaaS), anonymous visitors don't create person profiles. If a feature flag targets users by person properties AND that flag is evaluated on a pre-auth surface (landing page, pricing page, signup form), the anonymous user has no person profile for the flag to evaluate against, so the flag silently returns its default value. The variant the operator intended to ship to "users in the EU" / "users on the Pro plan" never reaches anyone visiting before login. This is a silent failure — the flag appears to work for identified users but the anonymous-traffic branch quietly never fires. + +Run **three** Greps in parallel: +- `person_profiles\s*:|personProfiles\s*:` — locate the person_profiles setting. +- `getFeatureFlag\(|isFeatureEnabled\(|useFeatureFlag\(|getFeatureFlagPayload\(` — every flag-eval call site. +- `posthog\.identify\(` — every identify call (used to classify a surface as pre-auth or post-auth). + +Step 1 — read the file(s) containing `person_profiles` hits to determine the configured value. If unset, the posthog-js default is `'identified_only'`. Record `mode` as `identified_only`, `always`, `never`, or `unset (defaults to identified_only)`. + +Step 2 — if mode is NOT `identified_only` (or unset), resolve `pass` with `details: "skip: person_profiles is not identified_only"` and return. + +Step 3 — for each flag-eval call site, read the surrounding file once. Classify it as **pre-auth** if it lives in: landing pages, marketing routes, pricing pages, signup/login UI components that render before the user authenticates, public homepage components, or any route gated to anonymous-only access. Classify as **post-auth** if the file also calls `posthog.identify()` in the same flow, requires authenticated session via middleware, or lives under a `/(app)/`, `/dashboard/`, `/(authenticated)/` style route. + +Step 4 — for each pre-auth flag-eval site, attempt to determine whether the flag's targeting condition references person properties. The subagent can't read PostHog flag definitions; instead, flag any pre-auth eval whose flag key suggests person-property targeting (variants gated on plan, country, persona, role, signup_method, etc.) — name patterns like `eu-banner`, `pro-only-cta`, `enterprise-pricing-variant`. When ambiguous, default to warning and let the operator confirm. + +Rule: +- pass: mode is not identified_only, OR no flag-eval call sites run on pre-auth surfaces, OR all pre-auth flag evals pass property overrides at eval time (`getFeatureFlag(key, { personProperties: {...} })` or equivalent). +- suggestion: 1–2 pre-auth flag-eval call sites exist but flag names don't strongly suggest person-property targeting — recommend the operator confirm flag definitions in PostHog. +- warning: 3+ pre-auth flag-eval call sites OR any pre-auth flag-eval whose flag name strongly suggests person-property targeting — anonymous users silently get default values. Recommend either passing property overrides at eval time, switching to `posthog.bootstrap.featureFlags` with server-computed values, or moving the eval behind authentication. + +Emit one `mcp__wizard-tools__audit_resolve_checks` call with a single update for id `ff-identified-only-pre-auth-targeting`, including `file` (path:line of the most representative pre-auth flag-eval) and `details` as compact JSON: + +``` +{ + "person_profiles_mode": "identified_only | always | never | unset", + "pre_auth_flag_eval_count": , + "examples": [ + {"file": "", "flag_key": "", "suspected_property_targeting": } + ] +} +``` + +Return when the call completes. Do not write the audit report. diff --git a/context/skills/audit-feature-flags/references/prompt-ff-local-eval-in-edge-handlers.md b/context/skills/audit-feature-flags/references/prompt-ff-local-eval-in-edge-handlers.md new file mode 100644 index 00000000..5cf21af5 --- /dev/null +++ b/context/skills/audit-feature-flags/references/prompt-ff-local-eval-in-edge-handlers.md @@ -0,0 +1,32 @@ +--- +description: Subagent prompt for ff-local-eval-in-edge-handlers — read only by its dispatched subagent, never by the main loop +--- + +You are an audit subagent. Resolve exactly one rule and return: ff-local-eval-in-edge-handlers. + +Read this skill's bundled `cutting-costs.md` reference once (typically `.claude/skills/audit-feature-flags/references/cutting-costs.md`; otherwise discover with `Glob` `**/skills/audit-feature-flags/references/cutting-costs.md`). Focus on the edge/Lambda callout — local evaluation in an edge or Lambda environment initializes a PostHog instance on every invocation, which defeats the polling cache and inflates cost drastically. For these environments, use regular flag evaluation, or share flag definitions via an external cache (see local-evaluation/distributed-environments). + +Run **two** Greps in parallel: +- `posthog\.init\(|new PostHog\(|posthog\.Posthog\(|Posthog\(` — every PostHog init site. +- `runtime\s*=\s*['"]edge['"]|export\s+const\s+runtime|export\s+const\s+config\s*=\s*\{[^}]*runtime|lambda|exports\.handler|handler\s*:\s*async\s*\(|app/api/.*/route\.(ts|js)` — edge / Lambda handler signals. + +Read each file that contains a PostHog init, once. For each init, classify whether the file is an edge handler (`runtime = 'edge'`, `app/api/*/route.ts` on Next.js edge runtime, Vercel/Cloudflare edge, Lambda handler shape `exports.handler` / `handler: async (event) =>`, or paths under `lambda/` / `edge/` / `functions/`). For each edge/Lambda file, check whether the init is configured for **local evaluation** (presence of `personal_api_key` / feature-flags secure key, or calls to `getAllFlagsAndPayloads` / `getAllFlags`). + +Rule: +- pass: no PostHog init runs in an edge / Lambda handler, OR every edge/Lambda init is configured for remote (non-local) evaluation. +- error: a PostHog init in an edge / Lambda handler is configured for local evaluation — per-invocation init negates the polling cache and inflates cost. +- warning: a PostHog init in an edge / Lambda handler has ambiguous configuration (e.g. reuses a shared init module that does configure local evaluation, but only some call sites are edge-runtime). + +Emit one `mcp__wizard-tools__audit_resolve_checks` call with a single update for id `ff-local-eval-in-edge-handlers`, including `file` (path:line of the offending edge/Lambda init) and `details` as compact JSON: + +``` +{ + "edge_init_count": , + "edge_local_eval_count": , + "examples": [ + {"file": "", "issue": "local-eval-in-edge | ambiguous-shared-init"} + ] +} +``` + +Return when the call completes. Do not write the audit report. diff --git a/context/skills/audit-feature-flags/references/prompt-ff-local-eval-polling-interval.md b/context/skills/audit-feature-flags/references/prompt-ff-local-eval-polling-interval.md new file mode 100644 index 00000000..0e60f114 --- /dev/null +++ b/context/skills/audit-feature-flags/references/prompt-ff-local-eval-polling-interval.md @@ -0,0 +1,31 @@ +--- +description: Subagent prompt for ff-local-eval-polling-interval — read only by its dispatched subagent, never by the main loop +--- + +You are an audit subagent. Resolve exactly one rule and return: ff-local-eval-polling-interval. + +Read this skill's bundled `cutting-costs.md` reference once (typically `.claude/skills/audit-feature-flags/references/cutting-costs.md`; otherwise discover with `Glob` `**/skills/audit-feature-flags/references/cutting-costs.md`). Focus on the "reducing local evaluation costs" section — by default, PostHog fetches flag definitions every 30 seconds. Each request is billed as 10 credits, so a constantly-running server makes `10 * 2 * 60 * 24 * 30 = 864,000` credits / month at the default. Increasing the polling interval (e.g. to 5 minutes) cuts that proportionally, at the cost of slower propagation of flag changes. + +Run **one** Grep: `featureFlagsPollingInterval|feature_flags_polling_interval|poll_interval|pollingInterval`. + +Read each file that contains a server SDK init, once (locate via the pre-flight local-eval signals if needed: `personal_api_key` / `PostHog(`). For each init that uses local evaluation, determine whether the polling interval (`featureFlagsPollingInterval` in posthog-node, `poll_interval` in posthog-python, or the language equivalent) is set. Request-timeout options (e.g. `featureFlagsRequestTimeoutMs`) are NOT polling-interval equivalents — do not count them as satisfying this rule. + +Rule: +- pass: every local-eval init sets the polling interval (or language equivalent) to a non-default value, OR sets it explicitly to the default with an intentional comment. +- suggestion: polling interval is unset (defaulting to 30s) — at constant load that's ~864k `/flags` credits / month. Recommend setting a larger interval (e.g. 300_000 ms / 5 min) if real-time flag updates are not required. +- warning: polling interval is set to a value **smaller** than the 30s default (e.g. 10s) — increases cost without operational benefit. + +Emit one `mcp__wizard-tools__audit_resolve_checks` call with a single update for id `ff-local-eval-polling-interval`, including `file` (path:line of the local-eval init) and `details` as compact JSON: + +``` +{ + "polling_interval_ms": , + "uses_default": true | false, + "estimated_monthly_credits": , + "examples": [ + {"file": "", "issue": "unset-default | sub-default"} + ] +} +``` + +Return when the call completes. Do not write the audit report. diff --git a/context/skills/audit-feature-flags/references/prompt-ff-test-ci-gating.md b/context/skills/audit-feature-flags/references/prompt-ff-test-ci-gating.md new file mode 100644 index 00000000..392046d9 --- /dev/null +++ b/context/skills/audit-feature-flags/references/prompt-ff-test-ci-gating.md @@ -0,0 +1,39 @@ +--- +description: Subagent prompt for ff-test-ci-gating — read only by its dispatched subagent, never by the main loop +--- + +You are an audit subagent. Resolve exactly one rule and return: ff-test-ci-gating. + +Read this skill's bundled `cutting-costs.md` reference once (typically `.claude/skills/audit-feature-flags/references/cutting-costs.md`; otherwise discover with `Glob` `**/skills/audit-feature-flags/references/cutting-costs.md`). Focus on the "configuring test and CI environments" section — test runners, CI pipelines, and staging environments often don't need real-time flag evaluation but silently rack up `/flags` requests on every init. The recommended pattern is to detect the test/CI environment (`process.env.NODE_ENV === 'test'`, `process.env.CI`, `BuildConfig.DEBUG`, etc.) and either skip init, disable flags (`advanced_disable_feature_flags: true`), or bootstrap deterministically. + +Run **three** Greps in parallel: +- `posthog\.init\(|new PostHog\(|posthog\.Posthog\(|Posthog\(` — every PostHog init site. +- `NODE_ENV.*test|process\.env\.CI|ProcessInfo\.processInfo\.environment\["CI"\]|BuildConfig\.DEBUG` — test/CI detection signals near init. +- `jest\.config|vitest\.config|playwright\.config|cypress\.config|\.test\.|\.spec\.|__tests__|tests/` (use `output_mode: "files_with_matches"`) — does the project have a test runner at all? + +If the third grep returns zero hits, resolve `pass` with `details: "skip: no test runner detected"` and return — this rule only applies to projects that actually run tests. + +Otherwise, read each file that contains a PostHog init, once. For each init, determine whether it is gated by a test/CI check: +- An `if (process.env.NODE_ENV !== 'test')` guard around the whole init call. +- An `advanced_disable_feature_flags: true` (or `preloadFeatureFlags: false`) conditional spread into the options when in test/CI. +- An early-return / `null` SDK shim in test mode. + +Rule: +- pass: every PostHog init is either gated against test/CI, or disables flags / preloading in test/CI, or the project's test runner setup makes the init unreachable in tests (e.g. a global setup file that monkey-patches PostHog). +- suggestion: init is unconditional but the project's tests do not appear to exercise it heavily (1–2 test files importing the init module). +- warning: init is unconditional and the project's test suite has 3+ files that load it — each test run silently incurs `/flags` requests. + +Emit one `mcp__wizard-tools__audit_resolve_checks` call with a single update for id `ff-test-ci-gating`, including `file` (path:line of the init that lacks gating) and `details` as compact JSON: + +``` +{ + "init_call_count": , + "test_gated_count": , + "test_runner_detected": true | false, + "examples": [ + {"file": "", "issue": "unguarded-in-tests"} + ] +} +``` + +Return when the call completes. Do not write the audit report. diff --git a/context/skills/audit-feature-flags/references/remediation.md b/context/skills/audit-feature-flags/references/remediation.md new file mode 100644 index 00000000..de57c8ad --- /dev/null +++ b/context/skills/audit-feature-flags/references/remediation.md @@ -0,0 +1,53 @@ +# Feature Flags Doctor — Remediation + +Read this file at Phase 2 (to classify findings) and again in Phase 3 (to apply them). Nothing in it is applied until the user selects the finding in the confirm step. + +How to fix each check's finding. Apply a fix **only** for findings the user selected in the confirm step, and make only the change described here. + +Each finding has a **fix type**: + +- **code** — edit the user's project (init options, flag call sites, env wiring via wizard-tools). +- **settings** — change PostHog state via the MCP flag-mutation tool. If no such tool is available, record it as manual guidance instead — do not fabricate a tool call. +- **manual** — environment- or app-specific. The doctor explains it in the report; the user acts. Not offered in the `wizard_ask` multi-select. + +## The interlock, restated + +When `ff-evaluated-not-reported` resolved as `warning` or `error` (`gates_cleanup: true`), every **settings** fix in this file is withheld from the multi-select — regardless of what other checks found. Staleness signals are computed from `$feature_flag_called` events; while those events aren't verifiably flowing, "unused" cannot be distinguished from "heavily used but unreported", and archiving on bad data turns off live features. Offer the `ff-evaluated-not-reported` code fix first; tenant cleanup happens on a future run once events are flowing. + +## The safe cleanup order (from the cleaning-up-stale-flags guide) + +A 100% rollout does not mean a flag is safe to disable — if deployed code still checks it, disabling turns the feature off for everyone. Therefore: + +- Flags **still referenced in code** (`stale-` rows): the doctor's fix is the CODE half only — remove the gate, keep the winning path. The report then instructs: deploy, verify, and only then disable the flag in PostHog. The doctor never disables a referenced flag. +- Flags with **zero code references** (`ff-active-but-unreferenced`): nothing checks them, so tenant-side archive/disable is safe immediately — with consent, and only when the interlock allows. + +## Mapping + +| Check / row | Fix type | Action | +| --- | --- | --- | +| `ff-key-authenticates` (missing/wrong key) | code | Wire the correct project token via `set_env_values`; never write `.env` directly. | +| `ff-key-authenticates` (personal key in client code) | manual | Explain the exposure; the user must rotate the key in PostHog and move server-side calls behind a backend. Never attempt the rotation. | +| `ff-flags-endpoint` (broken proxy route) | manual | Name the failing path + status; link the proxy docs. Proxy config is infrastructure — no automatic change. | +| `ff-flags-delivered` (parent row) | manual | No separate fix — the parent row summarizes counts; remediation lives on its per-flag `delivered-` rows. | +| `delivered-` rows | manual | Per-flag explanation with the `reason` code and what it means; link troubleshooting docs. Targeting/rollout intent is the operator's call. | +| `ff-unknown-flags` (parent row) | manual | No separate fix — the parent row summarizes counts; remediation lives on its per-flag `ghost-` rows. | +| `ghost-` rows | code | If a near-match roster key exists (report named it): fix the typo at the call site(s). If no similar flag exists: remove the dead call site's gate conservatively (keep the fallback path) or, when the surrounding code is non-trivial, downgrade to manual with the exact locations. | +| `ff-evaluated-not-reported` | code | Remove/condition the suppression so `$feature_flag_called` flows in production (keep legitimate test/CI gating intact). This fix is always offered FIRST when the interlock is active. | +| `ff-eval-before-identify` | code | Add re-evaluation after identify: subscribe via `onFeatureFlags` for the affected surface or call `reloadFeatureFlags()` after `identify()`. Minimal change; don't restructure auth flows. | +| `ff-bootstrap-when-known-set` | code | Set `bootstrap.featureFlags` from the known initial set at init. | +| `ff-await-readiness` | code | Gate the offending eval behind `onFeatureFlags`/`loaded`, or add bootstrap. One call site at a time. | +| `ff-default-values` | code | Add `?? ` at bare consumption sites. Choose the default that matches the pre-flag behavior (usually the fallback/control path). | +| `ff-bootstrap-distinct-id-mismatch` | code | Stabilize the bootstrapped id (use the value later passed to identify) or remove `bootstrap.distinctID` when no stable id exists pre-auth. | +| `ff-identified-only-pre-auth-targeting` | manual | Explain the anonymous-profile gap; options (property overrides at eval, server-computed bootstrap, move behind auth) are product decisions. | +| `stale-` rows | code | Strip the gate, keep the enabled/winning path (multivariate: keep the winning variant), remove dead imports/branches. Report adds the deploy-then-disable instruction. NEVER touch the flag in PostHog. | +| `ff-active-but-unreferenced` | settings | Archive (preferred) or disable each selected flag via the MCP flag-mutation tool. Gated by the interlock. If no mutation tool is available, list the exact flags + link the flags page as manual guidance. | +| `ff-local-eval-polling-interval` | code | Set `featureFlagsPollingInterval` (or language equivalent) to ≥300000 ms with a one-line comment stating the tradeoff. | +| `ff-local-eval-in-edge-handlers` | manual | Explain per-invocation init cost; regular evaluation or an external definitions cache are architecture choices. | +| `ff-test-ci-gating` | code | Guard init (or spread `advanced_disable_feature_flags: true` / `preloadFeatureFlags: false`) under the project's existing test/CI detection pattern. | + +## Per-fix rules + +- **One finding, one minimal change.** Never batch unrelated edits into one fix. Never reformat surrounding code. +- **Code removals are conservative.** When stripping a gate, if the else-branch contains logic that is not obviously dead (side effects, cleanup, telemetry), downgrade to manual and say why in the report. +- **Settings mutations name exactly what changed** — flag key + old state → new state — in "Fixes applied". +- **Environment fixes** go through `set_env_values` only, and the report never prints the value. diff --git a/context/skills/audit-feature-flags/references/report-format.md b/context/skills/audit-feature-flags/references/report-format.md new file mode 100644 index 00000000..930d9b23 --- /dev/null +++ b/context/skills/audit-feature-flags/references/report-format.md @@ -0,0 +1,117 @@ +# Feature Flags Doctor — Report Format + +Read this file only when you are ready to write the report. + +Write the report to `posthog-feature-flags-report.md` at the project root. + +## Required structure + +```markdown +# PostHog Feature Flags Doctor + +_Run: _ +_Project: _ + +## Summary + +- **Errors:** N +- **Warnings:** N +- **Suggestions:** N +- **Fixes applied:** N +- **Checks passed:** N +- **Checks skipped:** N + +## Delivery snapshot + +| Flag | In PostHog | Delivered | Evaluates to | Reason | +| --- | --- | --- | --- | --- | +| `checkout-v2` | active, 50% rollout | yes | `false` | out_of_rollout_bound | +| `new-nav` | active, 100% | yes | `true` | condition_match | +| `beta-search` | **not found** | — | `undefined` forever | ghost key | + +(One row per flag referenced in code or active in PostHog. This table is the heart of the report — it answers "why isn't my flag working?" per flag, in one line each.) + +## Findings + +### + +- **Severity:** error | warning | suggestion +- **Check:** +- **Affected:** +- **Evidence:** + - + - +- **Why it matters:** +- **Outcome:** <"Fixed — "> | <"Manual — "> | <"Withheld — cleanup gated until evaluation events are verified (see below)"> + +(Order: errors, then warnings, then suggestions.) + +## Fixes applied + +- ✓ — (code | settings) + +(If none: "No fixes were applied.") + +## Manual follow-up + +- — + +## Cleanup gated by the interlock + +(Include ONLY when `ff-evaluated-not-reported` was a finding.) + +Flag staleness in PostHog is measured by `$feature_flag_called` events, and this project is not reliably sending them — so "unused" and "heavily used but unreported" are currently indistinguishable. The following cleanup candidates were found but their PostHog-side fixes were withheld: + +- — + +Fix evaluation reporting first (see Findings), let a few days of data accumulate, then re-run the doctor. + +## Notes on expected behavior + +(Always include when relevant; these are teaching notes, not findings.) + +- **Automated browsers receive zero flags by design.** PostHog filters clients that look automated (headless browsers, crawlers, test runners) at the `/flags` endpoint — they get `{"errorsWhileComputingFlags": false, "flags": {}}` with HTTP 200. If you verify flags with Playwright/Cypress/Puppeteer, that's the one client guaranteed to see nothing. Test with a real browser profile or override the user agent + `navigator.webdriver` signals. +- + +## Checks passed + +- ✓ — + +## Checks skipped + +- ✗ — + +## Next steps + +<2–3 sentences pointing at the most impactful remaining item — or, on a clean bill, a note that delivery is verified end-to-end and a pointer to the flags best-practices doc.> +``` + +## Doc links per check + +- `ff-bootstrap-when-known-set`, `ff-bootstrap-distinct-id-mismatch` → https://posthog.com/docs/feature-flags/bootstrapping +- `ff-await-readiness`, `ff-default-values`, `ff-identified-only-pre-auth-targeting`, `ff-eval-before-identify` → https://posthog.com/docs/feature-flags/best-practices +- `ff-key-authenticates`, `ff-flags-endpoint`, `ff-flags-delivered`, `ff-unknown-flags`, `delivered-*`, `ghost-*` → https://posthog.com/docs/feature-flags/troubleshooting +- `ff-evaluated-not-reported` → https://posthog.com/docs/experiments/exposures +- `stale-*`, `ff-active-but-unreferenced` → https://posthog.com/docs/feature-flags/cleaning-up-stale-flags +- `ff-local-eval-polling-interval`, `ff-local-eval-in-edge-handlers` → https://posthog.com/docs/feature-flags/local-evaluation +- `ff-test-ci-gating` → https://posthog.com/docs/feature-flags/cutting-costs + +## Tone & content rules + +- **Be specific.** Flag keys, counts, status codes, `reason` codes. "`checkout-v2`: defined at 50% rollout, delivered, evaluates false (out_of_rollout_bound)" beats "some flags may not be delivered." +- **Teach in one line.** Every finding's "Why it matters" states the user-visible consequence (wrong variant, broken experiment exposure, silent default, wasted credits) — not the rule that fired. +- **No secrets, no PII.** Flag keys, hosts, paths, counts only. Never token values, distinct IDs, emails, or session IDs. +- **Round counts** above 1,000 (12,403 → 12.4k). +- **No emojis** beyond `✓` / `✗` list markers. + +## When there are zero findings + +Replace `Findings` with: + +```markdown +## Findings + +No issues found. Feature flag delivery is verified end-to-end and the integration looks healthy across all checks. +``` + +Keep the Delivery snapshot and `Checks passed` — on a clean bill they ARE the value: proof, per flag, that delivery works.