feat(feature-flags): reuse the basic-integration apps and grade the feature-flags outcome - #4189
Merged
Merged
Conversation
The wizard's layered refactor moves programs from src/lib/programs to src/programs. Look in both so e2e-only commands resolve on either side of it.
…on apps and grade its outcome Replace the two copied apps with pointer folders. A new .wizard-ci/source.json names the app a run copies, independent of expect.json, so reusing an app does not switch on the warehouse stub and checks. feature-flag-checks.ts grades a run against .wizard-ci/feature-flags.json: each flag exists, inactive at 0%, via the Flags API; each key sits in exactly one changed file, a constants module another changed file uses; no other example key appears; the report is written. CI never deletes the flags, because parallel legs share the keys.
The "constants module used by another changed file" check matched the module's bare name anywhere, so a comment or string saying "flags" passed it. It now requires an import form: a JS/TS specifier after from, require( or import(, or a Python import or from-import naming the module, including submodule, aliased, multi-name and commented parenthesized forms. The READMEs now list only what the checks grade, and say that existence cannot prove this run created a flag, because CI never deletes the shared keys. Directive: flag checks stay existence-only; the wizard reuses an existing flag, so a created_at gate would fail real runs Confidence: medium Scope-risk: narrow Not-tested: round-3 fix for `import os, posthog_flags` and commented parenthesized from-imports had no review after it (unit tests only)
…odule use Rails loads constants by name, so a controller using PostHogFeatureFlags::WIZARD_EXAMPLE_BACKEND has no import line, and the "constants module used by another changed file" check false-failed a correct Rails run. For a .rb constants file, a reference to the namespace enclosing the flag key now counts: its full path or innermost name, optionally with a leading ::. The enclosing namespace comes from indentation, not from counting block keywords: Ruby opens end-closed blocks in too many forms (mid-line begin, private def, x = if) for a keyword counter to stay right, and an outer wrapper namespace alone must never count. Constraint: assumes conventionally indented Ruby, which Rails and generated code are Confidence: medium Scope-risk: narrow Not-tested: round-3 switch to the indentation walk (memoized begin, private def cases) had no review after it; unit tests only Not-tested: no e2e rerun on a Rails app; the saved /tmp Rails run output was cleaned up before re-grading
johncwaters
marked this pull request as ready for review
September 24, 2026 17:05
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Feedback wanted: ready to merge. Approve or request changes. The direction is confirmed.
.wizard-ci/source.jsonthe right way to point an app at another app's tree, rather than growingexpect.json?Follows up on gewenyu99's two answers on #4136: reuse the
basic-integrationapps instead of copying them, and have the feature-flags e2e assert its own outcome.Changes
App pointer:
apps/<app>/.wizard-ci/source.json({"sourceApp": "..."}) names the app whose treerunE2ecopies. It is read only for that, so reusing an app does not depend onexpect.json, which would switch on the warehouse stub MCP and checks.source.jsonwins over the oldersourceAppinexpect.json.Pointer folders: the 123 copied files become two folders, each with a README,
source.jsonandfeature-flags.json:feature-flags/next-js/15-app-router-saaspoints atbasic-integration/next-js/15-app-router-saasfeature-flags/django/django3-saaspoints atbasic-integration/django/django3-saasFeature-flag checks:
services/wizard-ci/feature-flag-checks.tsgrades a run against the keys infeature-flags.json(both example keys for Next.js, the backend key only for Django):POSTHOG_PERSONAL_API_KEYwizard-example-*key appearsA changed file is one in the
/tmpcopy that differs from the source app. CI never deletes the flags, because parallel runs share the same keys.Wizard layout:
wizard-commands.tsfindse2e.jsonin bothsrc/programsandsrc/lib/programs, so e2e-only commands resolve on either side of the wizard's layered refactor.Follow-ups:
warehouse-seeded/next-stripe-declined'ssourceAppfromexpect.jsontosource.json. Left as is here.pnpm test:feature-flag-checksandpnpm test:source-appto.github/workflows/checks.yml. Not in this PR because the push token lacks theworkflowscope.How did you test this code?
pnpm test:feature-flag-checks(47 tests),pnpm test:source-app,pnpm test:warehouse-checksandtsc --noEmitpass.pnpm wizard-ci feature-flags/next-js/15-app-router-saas --e2e: pass, all 12 feature-flag checks green.pnpm wizard-ci feature-flags/django/django3-saas --e2e: pass, all 7 feature-flag checks green.878a5b7f: Next.js passes. Django failsno unexpected flag key, because that run added a frontend side (browser snippet intemplates/base.html,wizard-example-frontend-flagevaluated intemplates/dashboard/index.html) to an app whose templates have no scripts. The check is kept strict, and the side choice is fixed in the wizard: [CI] (48ac55f) tanstack-start/tanstack-start-saas #1322's seed prompt no longer counts templates without their own<script>as a frontend. With that prompt, Django passed twice (backend flag only, key indashboard/feature_flags.py) and Next.js passed (both flags).24de57a4e, wizard at [CI] (48ac55f) tanstack-start/tanstack-start-saas #13225e3c1c6e: Next.js, Django and Rails (basic-integration/rails/fizzy, through a temporary pointer that is not committed) all pass. Rails passes the constants-module check through the autoloadedPostHogFeatureFlags::BACKEND_EXAMPLEreference, with no import.feature-flags.jsonset to a wrong key (wizard-example-wrong-flag): fails as it should: the flag does not exist, the key is in no changed file, andwizard-example-backend-flagis reported as unexpected.LLM context
Written with Claude Code.