Skip to content

feat(audit): make feature flags a wizard native doctor program - #1064

Closed
ryanwaits wants to merge 10 commits into
PostHog:mainfrom
ryanwaits:feat/feature-flags-doctor
Closed

feat(audit): make feature flags a wizard native doctor program#1064
ryanwaits wants to merge 10 commits into
PostHog:mainfrom
ryanwaits:feat/feature-flags-doctor

Conversation

@ryanwaits

@ryanwaits ryanwaits commented Aug 6, 2026

Copy link
Copy Markdown

Problem

Pairs with PostHog/context-mill#329, which should land first.

audit feature-flags is skill-backed today, so it gets the generic run screen: three lines of boilerplate and a spinner. The rebuilt skill runs a 19-check pipeline and asks for consent before applying fixes.

Changes

Makes audit feature-flags wizard-native, the same move audit web-analytics made. The native handler shadows the published cliEntry, so the command and skill id are unchanged and there's no context-mill release coupling.

  • The program (feature-flags-doctor) — detect step, audit-family screens, and a 19-row seeded ledger whose ids are the contract with the skill. Sweep rows (ghost-*, stale-*) get appended at runtime per flag.
  • Slide registryAuditRunScreen picked its deck with a hardcoded skillId === 'audit-events' ternary, so any third audit program silently got the comprehensive deck. Replaced with a lookup keyed by skill, plus a five-slide feature-flags deck. Side effect: events-audit's flat-command form was hitting the same fallthrough and now gets its own deck too.
  • OAuth scopes — the doctor requests feature_flag:read/write. Native programs don't inherit agent-skill's additions, so without this the roster call 403s and every roster-dependent check silently degrades. Found the hard way on a live run.
  • Picker de-dupe — a native handler shadowing a published entry listed the command twice.
04-ledger-findings 03-consent-interlock

Test plan

  • pnpm build && pnpm test (1745, +15 new) and pnpm lint green.
  • Ran end-to-end against a Next.js app with seeded flags and three planted defects. All three caught, two selected fixes applied, cleanup correctly withheld, healthy flags verified. Same app on main: all three silent.

Notes / open questions

Three things I'd like a maintainer opinion on rather than deciding unilaterally:

  1. The ledger has no "fixed" state. Approve a fix, watch it apply, and the row stays red — AuditStatus has no way to say "was a finding, now handled" I've added this to a list of follow ups for later, but thoughts are welcome. web-analytics's findings JSON already models this (fix.applied), so it's parity rather than a new concept.
  2. Parent + sweep rows double-count in the outro — "Code flag keys exist in PostHog" and "beta-serach not found in PostHog" are one problem shown twice. Resolve the parent to pass, or collapse children in the summary?
  3. Area naming — kept the incumbent's Feature Flags — Optimize prefix. Bare verbs read better standalone, imo; happy to flatten.

Also spotted but left alone: AuditChecksOutroSection renders "1 warnings", and its counts ignore fixes applied during the run. Both pre-existing and family-wide — can drive-by if wanted.

@ryanwaits
ryanwaits requested a review from a team as a code owner August 6, 2026 19:04
@gewenyu99

Copy link
Copy Markdown
Collaborator

General discussion points, not feedback:

  • So one of the reasons we wanted audit to be read only was so it's safe to throw to someone and have them run it. Nothing changes, just a report, it's great that you preserved this
  • I kinda dig the name doctor, what's your vision on this project, would you rename audit to doctor and move all the commands? What would be the useful full scope here?
  • Feature flag write scope kinda scares me. There are certain scopes I really get nervous about giving the Wizard, and flags write is definitely one of them. Out of scope today, but how might you approach this potentially scary issue?

@edwinyjlim

Copy link
Copy Markdown
Member

The audit is stuck seeding the audit checklist for me. But you've plugged into the audit program's pattern, which is a bit of a tangled mess on our part, successfully.

I'd think about how all this information is presented. It's a massive check list. Ideally, we'd have the developer engaged with what we're diagnosing and solving for them

@sarahxsanders sarahxsanders left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice job getting a wizard program working!!! some high level bits:

  • per-program OAuth scopes is a nice touch :) I like the mindfulness here for security
  • at the default terminal size (I use ghostyy) it's cutting off a bunch of text for me in the checks. hecks made me lol
Image - run takes a bit of time, explore around and see how you can possibly trim down the time it takes. don't get lost on time doing it if it isn't superrrr obvious though! a working program matters :) - I'd love to see more progressive disclosure in the learn cards, the nice thing with the cards is they capture more attention when they're not tied to a specific step. don't spend all your time here if you're running out, but something to note - idk if it was just me, but I don't see the Delivery and Observability learn cards when I run it

for stuff you can't get to today, would love to see in your reflections how you'd approach it!

@gewenyu99

Copy link
Copy Markdown
Collaborator

Generally does what I'd expect it to, two more pieces of general feedback, again for discussion:

  • How would you benchmark this, how would you measure this program's success with telemetry and without breaching privacy? (ideas are fine there is no right answer)
  • What do you think is missing in terms of engaging the user. They're gonna be stuck waiting for the audit for a while, regardless of how fast models become. What should we do with their time? or should we tell them to go grab a coffee?
  • User interactions come late into the run. This makes them very very missable. How do you think we can make it obvious that interactivity will come, and if they accidentally kill a run by closing a terminal, could we recover?
  • The program's checklist is static. The dreams for us is to have a more dynamic process to enqueue audit items. how might this fit into that picture, what might you have to change?

@ryanwaits

Copy link
Copy Markdown
Author

pushed fixes for everything concrete — quick rundown, then the discussion answers:

fixed in the latest push

  • cut-off checks (@sarahxsanders): wouldve never caught that one lol, but now its height-aware (wrote a couple tests too so lmk if that actually fixed it!)
  • missing delivery/observability cards: yeah this was a bug, it now walks one card per ~12s min dwell, with a test replaying the exact batch from a logged run
  • speed: landed the cheap trim for now (prompt asserts detected SDKs/token/check ids so the agent stops re-deriving them); prefetching the flag roster + a single-pass checks rewrite shaved a few min off with the local test, but this is something i can follow up on as well (or a longer reflection note, move the doctor/write capabilities to its own separate flag with scoped categories)

@gewenyu99 ive got a follow up item list im working through so ill get to your discussion items once i finish up a few other items, but a few of these were already on it!

@ryanwaits

Copy link
Copy Markdown
Author

@gewenyu99 grabbing the ones i have actual opinions on:

audit doctor. i dont think id rename, i'd split. audit is the report you hand to anyone and nothing happens. doctor is the same checks but it offers to fix. two ways into one skill, so the read-only version stays read-only (also discussed potentially a flag of sorts in the cli?)

flags write. yeah i'm not comfortable with it either. archiving the wrong flag is quiet, nobody finds out for a while. couple things i've been kicking around: ask for archive specifically instead of write, since archive is all i actually do with it. or don't ask at login at all, ask at the moment someone approves a cleanup? so nobody's handing over write for a report they were never going to act on.

the wait + interactivity showing up late. this was on my follow up list, cards should be paged with a keypress to advance too (i think thats broken across the board, pre existing?) pressing a key is the engagement, and it makes it obvious the thing is interactive well before we need them

on recovery: the checklist is already written to disk as it goes, but i think the thing worth saving is what the agent found. save that incrementally and a killed run comes back with "here's what we found last time, skip to the fixes?"

@edwinyjlim edwinyjlim closed this Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants