Skip to content

fix(feature-flags): follow the Ready for cleanup rule in the flag cleanup skills (EMSR-2643) - #240

Open
fsaresh wants to merge 9 commits into
mainfrom
fsaresh/emsr-2643-cleanup-readiness
Open

fsaresh wants to merge 9 commits into
mainfrom
fsaresh/emsr-2643-cleanup-readiness

Conversation

@fsaresh

@fsaresh fsaresh commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary

The flag cleanup and flag discovery skills treated a flag that is OFF in every critical environment as ready to remove, and they told agents to hardcode its offVariation. A flag that is off everywhere often guards a feature that was never released, so the cleanup deletes that feature. LaunchDarkly's Ready for cleanup view does not list this flag: a flag is ready for code removal only when it is launched (ON, serving one variation, evaluated and unchanged recently) in every critical environment.

Fix

  • Both skills now state the Ready for cleanup rule, with the defaults from the project lifecycle settings (30 days since creation, 7 days for evaluations and changes).
  • A flag that is off everywhere is not ready for code removal. The cleanup skill continues only when the user confirms that the code behind the other variations is abandoned, and it records that confirmation in the PR description.
  • The skills read the new details.lifecycle verdict from the removal readiness tool:
    • blocked with not_ready_for_code_removal: do not remove code, and name the environments that are not launched.
    • readyToArchive: true: suggest archiving the flag, not code removal.
    • codeAlreadyRemoved: true: the flag is launched everywhere and has no code references left, so suggest archiving it.
    • caution is no longer permission to remove code. For code_removal_readiness_unavailable, the user must confirm in LaunchDarkly that the flag is launched.
  • Both skills move to 1.1.0-experimental.

Testing

  • python3 scripts/validate_skills.py passes (54 skills).
  • python3 scripts/generate_catalog.py makes no change to skills.json.
  • pytest tests/*.py passes.

Related

🤖 Generated with Claude Code

Opened from termag


Note

Overview
Flag cleanup and discovery skills (1.1.0-experimental) no longer treat a flag that is OFF in every critical environment as safe to hardcode via offVariation. They now match LaunchDarkly’s Ready for cleanup rule: code removal requires the flag to be launched everywhere (targeting ON, single variation, recent evaluation, stable targeting), not merely inactive or off with a shared offVariation.

Both skills consume get-removal-readiness → details.lifecycle: honor readyForCodeRemoval, route readyToArchive to archive-not-remove, handle codeAlreadyRemoved with post-deploy archive guidance, and treat caution as requiring explicit user confirmation (including when lifecycle data is unavailable). Discovery splits ready for code removal vs ready to archive and drops advice to turn flags off before removal.

Reference docs, PR template example, catalog versions, and CHANGELOG are updated; a small unit test locks the decision matrix so “never requested” and “off everywhere” rows say not ready for code removal.

Reviewed by Cursor Bugbot for commit ac6761d. Bugbot is set up for automated code reviews on this repo. Configure here.


Related Jira issue: EMSR-2643: [Claude] Flag cleanup skill treats flags that are off everywhere as ready for removal; the UI's Ready for cleanup view does not

fsaresh and others added 3 commits November 4, 2026 22:25
…ls (EMSR-2643)

The flag cleanup skill treated a flag that is OFF in every critical
environment as safe to remove, and hardcoded its offVariation. The
LaunchDarkly Ready for cleanup view does not: a flag is ready for code
removal only when it is launched in every critical environment, and
launched requires targeting to be ON (gonfalon
internal/flags/flag_staleness.go getStaleFlagDataWithLifecycleSettings,
internal/flags/models/feature_flag.go FullyRolledOutVariation).

- launchdarkly-flag-cleanup 1.1.0-experimental: state the product rule,
  stop on off-everywhere flags, and continue only on explicit user
  confirmation that the other variations' code is abandoned.
- launchdarkly-flag-discovery 1.1.0-experimental: split "ready for code
  removal" from "ready to archive", and stop telling users to turn a
  flag off before removal.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Generated-by: termag
…ness (EMSR-2643)

check-removal-readiness now reads LaunchDarkly's code removal verdict
(flag-archive-checks) and returns it in details.lifecycle. A flag that is
not ready for code removal is "blocked" with a not_ready_for_code_removal
blocker, and the verdict is "caution" with code_removal_readiness_unavailable
when the tool cannot read it. The skills said "safe" means proceed and
"caution" means let the user decide, which an agent can read as permission
to delete code.

- launchdarkly-flag-cleanup 1.2.0-experimental: on "blocked" with
  details.lifecycle.readyToArchive true, suggest archive-flag and do not
  remove code; treat "caution" as needing user confirmation; name the
  environments that are not launched.
- launchdarkly-flag-discovery 1.2.0-experimental: the same rules in the
  verdict list and the removal readiness checklist.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Generated-by: termag
…p (EMSR-2643)

check-removal-readiness now reports details.lifecycle.codeAlreadyRemoved for a
flag that is launched in every critical environment and has no code references
left. Gonfalon does not mark that flag ready for code removal, because it needs
code references. The cleanup and discovery skills now suggest archiving it.

Set both skills to 1.1.0-experimental: this branch is one release.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Generated-by: termag
@github-actions

github-actions Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Skill eval results

Skill Before After Δ
agentcontrol/configs-create 100/100 (4/4) 75/100 (3/4) -25
agentcontrol/configs-update 80/100 (4/5) 80/100 (4/5) no change
agentcontrol/configs-variations 80/100 (4/5) 80/100 (4/5) no change
agentcontrol/tools 75/100 (3/4) 75/100 (3/4) no change
aws-devops-agent/experiment-orchestration - 100/100 (9/9) new
experiments/launchdarkly-experiment-guide - 100/100 (5/5) new
experiments/launchdarkly-experiment-hypothesis-builder - 100/100 (13/13) new
experiments/launchdarkly-experiment-setup - 100/100 (8/8) new
feature-flags/flag-and-release-change - 100/100 (4/4) new
feature-flags/flag-release - 80/100 (4/5) new
feature-flags/launchdarkly-flag-command - 100/100 (3/3) new
feature-flags/launchdarkly-flag-create 100/100 (3/3) 100/100 (4/4) no change
feature-flags/launchdarkly-flag-drift - 100/100 (4/4) new
feature-flags/should-flag-change - 100/100 (17/17) new
onboarding - 86/100 (6/7) new
release-autopilot/launchdarkly-release-autopilot-onboarding - 100/100 (25/25) new

Only suites whose source actually changed since their last recorded score were re-run. Soft-failing while we stabilise the baseline.

…g cleanup skills (EMSR-2643)

The flag cleanup and discovery skills named the gram-functions tools
(check-removal-readiness, get-flag, list-flags, find-stale-flags,
get-code-references). gram-functions is being retired. The skills now use the
hosted LaunchDarkly MCP names: get-removal-readiness, get-flags,
search-stale-flags and get-flag-code-references. The get-flags state filter
now lists the hosted values (live, deprecated, archived).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Generated-by: termag

For a `not_ready_for_code_removal` blocker, tell the user why LaunchDarkly does not consider the flag ready for code removal. The blocker detail and `details.lifecycle.environments` name the critical environments where the flag is not `launched`.

**If `details.lifecycle.readyToArchive` is `true`, suggest archiving the flag instead.** LaunchDarkly considers the flag inactive in every critical environment and without code references. Offer to archive it with `archive-flag`. Do not remove code and do not hardcode a variation. If you found references in Step 1, tell the user that LaunchDarkly reports no code references for this flag, so code references may not be set up for this repository.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

RG Review Desk · Valerie lens
Team-specific automated review · separate from Cursor Bugbot

Blocking — archival is offered when the evidence says it is unsafe

This offers archive-flag when readyToArchive is true even if local code references were found, and when codeAlreadyRemoved is true without requiring readyToArchive. Both states are reachable. An archived flag is treated as absent, so any remaining evaluation gets the code fallback instead of the configured variation.

Do not offer archival while local references remain, or while readyToArchive is false.

🤖 Powered by Cursor

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 2083d37. The skill no longer offers archive-flag when Step 1 found references. For codeAlreadyRemoved (now caution from the MCP tool, mcp 7f95cc0), it offers archival only after the user confirms the cleanup is deployed everywhere. readyToArchive is false for every launched flag, so requiring it would block the normal archive after a cleanup.

The tool returns a readiness verdict:

**`safe`**: No blockers or warnings. Proceed with removal.
**`safe`**: No blockers or warnings, and LaunchDarkly marks the flag ready for code removal (`details.lifecycle.readyForCodeRemoval` is `true`) or reports that its code is already removed (`details.lifecycle.codeAlreadyRemoved` is `true`). Continue to the "Ready for cleanup" check below.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

RG Review Desk · Valerie lens
Team-specific automated review · separate from Cursor Bugbot

Blocking — the fallback cannot verify lifecycle readiness

Before the MCP lifecycle fields exist, this tells the agent to use get-flags to confirm recent SDK evaluation and whether every critical environment is launched. That tool returns the targeting tree, not lifecycle status or recent evaluations. When lifecycle data is present, the same section still forces the default 30/7-day, temporary, and prerequisite checks and says to stop even if readyForCodeRemoval is true, including when the project has changed or disabled those checks.

The agent can invent the missing evidence, allow a cleanup it cannot verify, or reject a flag the project’s own policy marks ready.

Do not treat get-flags as proof of lifecycle or evaluation facts. When those fields are missing, stop. When they are present, follow the project’s returned verdict instead of reimposing the default checks.

🤖 Powered by Cursor

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 2083d37. The skill now follows details.lifecycle.readyForCodeRemoval and does not apply the default checks again. It uses get-flags only to read targeting. When the lifecycle verdict is missing or unavailable, it stops and asks the user to check the Ready for cleanup view.

…roject's cleanup verdict (EMSR-2643)

The cleanup skill offered archive-flag when readyToArchive was true even if the
repository still referenced the flag. An archived flag is absent, so that code
gets its fallback value. The skill now offers archival only when Step 1 found no
references. For codeAlreadyRemoved, which the MCP tool now returns as caution,
it offers archival only after the user confirms that the cleanup is deployed
everywhere.

The skill told agents to confirm evaluation recency and launched status with
get-flags, which returns only targeting. It also applied the default lifecycle
checks again on top of readyForCodeRemoval. The skill now follows
readyForCodeRemoval. When the lifecycle verdict is missing or unavailable, it
stops and asks the user to check the Ready for cleanup view.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Generated-by: termag

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

fsaresh and others added 2 commits November 5, 2026 00:23
…es reach code removal (EMSR-2643)

When codeAlreadyRemoved was true and Step 1 found local references, the skill
said to continue, and then stopped on readyForCodeRemoval false. That value is
false only because LaunchDarkly does not scan the repository. The stop now has
an exception for this case.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Generated-by: termag
Keep the hosted MCP tool names from #231 and this branch's readiness
wording. Drop the tool-name clause from this branch's CHANGELOG entry,
because #231 made that change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Generated-by: termag

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ 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 49b301c. Configure here.

fsaresh and others added 2 commits November 5, 2026 23:27
…oval (EMSR-2643)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Generated-by: termag
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Generated-by: termag
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.

1 participant