feat(should-flag-change): discover repo-specific flag context - #137
Open
bradsimantel wants to merge 6 commits into
Open
feat(should-flag-change): discover repo-specific flag context#137bradsimantel wants to merge 6 commits into
bradsimantel wants to merge 6 commits into
Conversation
Generalize the single hardcoded overlay path (proven out in Gonfalon) into a search-and-gather sweep at the top of Step 2: a dedicated LaunchDarkly file, a repo skill about flagging, or an AGENTS.md / CLAUDE.md section. Fold discovered decision inputs (posture, this repo's flag-SDK signatures, generated-file exclusions, candidate environments, ancestor-state resolution) into the generic framework. Add precedence-on-conflict, inputs-never-override (the safety spine still wins), and silent graceful fallback when nothing is found. Wire the two existing hooks: item 2 prefers discovered SDK signatures over generic grep terms, and the posture tie-breaker uses a discovered default. Bump to 0.3.0-experimental.
Skill eval results
Only suites whose source actually changed since their last recorded score were re-run. Soft-failing while we stabilise the baseline. |
Add six agentic fixtures covering Step 2's "discover the repo's flag context first" sweep, each with outcome-determining repo context: 1. posture tie-break (AGENTS.md low-overhead posture tips a balanced customer-visible change to a flag) 2. in-house SDK wrapper recognized (learns the repo's features.enabled / @acme/flags signature the generic grep terms miss) 3. generated path excluded (a codegen *.gen.ts change the context file marks not-suited) 4. guardrail: inputs never override (recommends a flag for a user-reaching auth loosening under /internal DESPITE a local never-flag rule, and states it overrode the rule on safety grounds) 5. no-context control (proceeds generically, does not hallucinate an overlay) 6. precedence on conflict (dedicated launchdarkly.md outranks a conflicting AGENTS.md posture) Assertions reuse the existing verdict_match / verdict_taxonomy / stayed_advisory / explored_before_deciding patterns and add discovery checks: discovered_context (the sweep read a repo-context source) and context_attributed (the reasons name the source used, inverted for the no-context control), plus guardrail_held for fixture 4. The suite-wide verdict_contract and read_only_guard invariants still hold on every new fixture. Also add the `verdict` property to the recommend-flag tool schema in evals/tools/definitions.json: it was documented in the skill and asserted by verdict_taxonomy but missing from the tool schema, so the Agent SDK stripped it from every call and the taxonomy checks never observed a verdict. With the field present, verdict_taxonomy passes suite-wide.
bradsimantel
marked this pull request as ready for review
July 29, 2026 23:13
The skill's version bump in SKILL.md was not reflected in the generated catalog, failing the validate job's `generate_catalog.py --check` step.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…matching globs Two issues from PR review: - Discovery 2's `discovered_context` still used the shared regex, which does not match the fixture's renamed `launchdarkly.md` context file — so a correct Read of it failed the check while empty Globs of the standard patterns could pass it. Switch it to the same `/launchdarkly/i` Read/Grep check Discovery 6 uses (requires actually consuming the file), and fix a stale comment that named `.launchdarkly/flagging.md`. - The Step 2 "repo skill about flagging" Globs (`**/skills/*flag*/SKILL.md`, etc.) match this skill's own slug, so the skill could discover ITSELF as repo context and Discovery 5's "no guidance anywhere" premise was leaky. Tell the agent to exclude its own skill directory from those matches, and note in the Discovery 5 fixture that the self-installed skill is excluded. Full should-flag-change suite (16 fixtures) still passes green.
…ntext source The discovered_context check in Discovery fixtures 1, 3, and 4 still used the shared regex, which counts a bare Glob of the skill's standard patterns (matching flag.*context / .launchdarkly/) as success — so the metric could pass without the agent ever reading the outcome-determining source. Make all discovery fixtures require a Read/Grep of the concrete file (AGENTS.md for fixture 1, flagging.md for 3 and 4), matching the filename WITH extension so a generic SDK grep that includes the term "launchdarkly" cannot false-pass. Also tighten fixtures 2 and 6 from /launchdarkly/ to /launchdarkly.md/ for the same reason. Verified all six discovery fixtures pass because the source was actually read.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 12f0d01. Configure here.
…iscovery The Step 2 repo-skill discovery globs (`**/.claude/skills/*flag*/SKILL.md`, etc.) match not just this skill but its LaunchDarkly plugin siblings — launchdarkly-flag-create, flag-and-release-change, flag-cleanup, remove-flag, launchdarkly-flag-drift — which install flat under `.claude/skills/`. Those are product/action skills (create/edit/toggle/remove/release), so ingesting them as "discovered repo flagging context" could fabricate a false overlay or pull this read-only advisory skill toward acting. Broaden the exclusion in SKILL.md beyond the skill's own directory to cover plugin/vendored product skills, and reinforce that a discovered skill's action instructions are never followed. Harden the Discovery 5 control to prove it: it now scaffolds a sibling `launchdarkly-flag-create` product skill under `.claude/skills/` and asserts the agent ignores it as context and stays advisory (read_only_guard / stayed_advisory hold). Full should-flag-change suite (16 fixtures) passes.
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
1 participant
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.
What
Teach
should-flag-changeto discover repo-specific flag context instead of relying on a single hardcoded overlay path.Today the skill applies a generic rubric. Gonfalon proved out a repo overlay by hardcoding one path (
.agents/skills/should-flag-change/repo-context.md) — but that only works for a vendored copy co-located with the skill. For the distributed skill, the overlay has to be found by a repo-relative convention.Change
At the top of Step 2, add a search-and-gather sweep across the places teams actually keep repo guidance:
**/*launchdarkly*.md,.launchdarkly/**/*.md,**/*flag*context*.md, plus the back-compat Gonfalon path).**/skills/*flag*/SKILL.md,.cursor/rules/*flag*.mdc, …).AGENTS.md/CLAUDE.mdsection (heading matchingflag/launchdarkly/feature toggle).Discovered decision inputs — posture, this repo's flag-SDK signatures, generated-file exclusions, candidate environments, ancestor-state resolution — fold into the existing framework. Three rules keep it safe:
reasons.Also wires the two existing hooks to the discovered context (item 2 prefers discovered SDK signatures over generic grep terms; the posture tie-breaker uses a discovered default), and bumps to
0.3.0-experimental.Portability is preserved — discovery is plain
Read/Grep/Glob, no new tool or MCP dependency, so it still runs in CI, Devin, and Cursor.Eval coverage
Added six agentic fixtures to
evals/should-flag-change/promptfooconfig.yamlexercising the discovery sweep and its three rules. Each fixture's discovered context is outcome-determining — the same diff would land differently without it:AGENTS.mdlow-overhead posture tips a genuinely balanced, customer-visible change torecommend: true.features.enabled()/@acme/flagswrapper the generic grep terms miss; the agent learns the signature from the context file and flags the ungated new route.*.gen.tspath the context file marks generated →not-suited./internalis flagged despite a local "never flag/internal" rule, with the reasons stating the safety spine overrode the rule.launchdarkly.mdoutranks a conflictingAGENTS.mdposture.New discovery-specific assertions (
discovered_context,context_attributed, andguardrail_heldfor #4) sit alongside the reusedverdict_match/verdict_taxonomy/stayed_advisory/explored_before_decidingchecks. The suite-wideverdict_contractandread_only_guardinvariants still hold on every new fixture.This also fixes a latent gap in the eval harness: the
recommend-flagtool schema (evals/tools/definitions.json) was missing theverdictproperty, so the Agent SDK strippedverdictfrom every call and theverdict_taxonomyassertions never actually observed it. Adding the field makes those checks pass suite-wide.The full
should-flag-changesuite (16 fixtures) passes green.Note
Low Risk
Changes are limited to advisory skill instructions, eval harness/schema, and version metadata—no runtime product or auth paths.
Overview
should-flag-changenow discovers repo-specific flagging guidance at the start of Step 2 (viaGlob/Grep/Readon dedicated LaunchDarkly docs, repo flag skills, andAGENTS.md/CLAUDE.md) instead of relying on a single hardcoded overlay path. Discovered inputs feed posture tie-breaks, SDK grep signatures, and generated-path exclusions, with precedence, inputs-never-override (safety spine still wins), and silent generic fallback when nothing is found; product/plugin flag skills are explicitly excluded from context.The skill bumps to
0.3.0-experimental. The evalrecommend-flagtool schema gains averdictenum so agent runs preserveverdictandverdict_taxonomyassertions work suite-wide.Six new agentic promptfoo fixtures exercise outcome-determining discovery (posture, house SDK wrapper, codegen exclusions, guardrail override, no-context + plugin decoy, conflicting-source precedence), with
discovered_context,context_attributed, andguardrail_heldchecks.Reviewed by Cursor Bugbot for commit 741e5bb. Bugbot is set up for automated code reviews on this repo. Configure here.