feat(audit): make feature flags a wizard native doctor program - #1064
feat(audit): make feature flags a wizard native doctor program#1064ryanwaits wants to merge 10 commits into
Conversation
|
General discussion points, not feedback:
|
|
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
left a comment
There was a problem hiding this comment.
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
- 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!
|
Generally does what I'd expect it to, two more pieces of general feedback, again for discussion:
|
|
pushed fixes for everything concrete — quick rundown, then the discussion answers: fixed in the latest push
@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! |
|
@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?" |
Problem
Pairs with PostHog/context-mill#329, which should land first.
audit feature-flagsis 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-flagswizard-native, the same moveaudit web-analyticsmade. The native handler shadows the publishedcliEntry, so the command and skill id are unchanged and there's no context-mill release coupling.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.AuditRunScreenpicked its deck with a hardcodedskillId === '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.feature_flag:read/write. Native programs don't inheritagent-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.Test plan
pnpm build && pnpm test(1745, +15 new) andpnpm lintgreen.main: all three silent.Notes / open questions
Three things I'd like a maintainer opinion on rather than deciding unilaterally:
AuditStatushas 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.Feature Flags — Optimizeprefix. Bare verbs read better standalone, imo; happy to flatten.Also spotted but left alone:
AuditChecksOutroSectionrenders "1 warnings", and its counts ignore fixes applied during the run. Both pre-existing and family-wide — can drive-by if wanted.