Skip to content
Merged
196 changes: 186 additions & 10 deletions products/feature_flags/mcp/tools.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,148 @@ tools:
enabled: false
feature-flag-archive:
operation: feature_flags_archive_create
enabled: false
enabled: true
scopes:
- feature_flag:write
annotations:
readOnly: false
destructive: false
idempotent: true
enrich_url: '{id}'
title: Archive feature flag
description: >
Archive a feature flag, hiding it from the default flag list. Requires the flag's numeric ID; use
`feature-flag-get-definition-by-key` to get it from a key, or `feature-flag-get-all` to search by key or
name.
Comment thread
posthog[bot] marked this conversation as resolved.
Resolved


Sets `archived` to true. An archived flag must be disabled, so an enabled flag is turned off in the same
call. Nothing else changes: this tool never accepts or replaces the `filters` object, and linked experiment
and survey history is preserved. There is no request body.


When the flag is enabled, archiving stops it evaluating everywhere. Tell the user what else this affects:
linked experiments, surveys, early access features and session replay settings all appear in the flag's full
definition.


Returns 400 when the flag is enabled and other active flags depend on it. Returns 409 when an approval
policy gates disabling. Archiving an already-archived flag succeeds and changes nothing, so retrying is
safe.


A key lookup that finds nothing does not mean the flag is absent. Archived flags are hidden from
`feature-flag-get-definition-by-key` and from `feature-flag-get-all`, so a flag that is already archived
looks missing on a retry. Pass `{"archived":"true"}` to `feature-flag-get-all` before you report the flag as
missing.


Use `feature-flag-unarchive` to put the flag back in the list, or `delete-feature-flag` when the user wants
it gone rather than tidied away.
param_overrides:
id:
cast: string-int
response:
include:
- id
- key
- active
- archived
- status
- version
feature-flag-disable:
operation: feature_flags_disable_create
enabled: false
enabled: true
scopes:
- feature_flag:write
annotations:
readOnly: false
destructive: false
idempotent: true
enrich_url: '{id}'
title: Disable feature flag
description: >
Turn a feature flag off. Requires the flag's numeric ID; use `feature-flag-get-definition-by-key` to get it
from a key, or `feature-flag-get-all` to search by key or name.


Sets `active` to false and changes nothing else. This tool never accepts or replaces the `filters` object,
so targeting, rollout percentages, variants and payloads cannot change. There is no request body.


A disabled flag stops evaluating everywhere, so every caller falls back to its own default. Tell the user
what else this affects: linked experiments, surveys, early access features and session replay settings all
appear in the flag's full definition. `feature-flag-get-definition-by-key` already returns it, and
`feature-flag-get-definition` returns it by ID.


Returns 400 when other active flags depend on this one; `feature-flags-dependent-flags-retrieve` lists them.
Returns 409 when an approval policy gates disabling. Disabling an already-disabled flag succeeds and changes
nothing, so retrying is safe.


A key lookup that finds nothing does not mean the flag is absent. Archived flags are hidden from
`feature-flag-get-definition-by-key` and from `feature-flag-get-all`, and an archived flag is already
disabled. Pass `{"archived":"true"}` to `feature-flag-get-all` before you report the flag as missing.


To turn the flag off at a future time instead, use `scheduled-changes-create` with the `update_status`
operation.
param_overrides:
id:
cast: string-int
response:
include:
- id
- key
- active
- archived
- status
- version
feature-flag-enable:
operation: feature_flags_enable_create
enabled: false
enabled: true
scopes:
- feature_flag:write
annotations:
readOnly: false
destructive: false
idempotent: true
enrich_url: '{id}'
title: Enable feature flag
description: >
Turn a feature flag on. Requires the flag's numeric ID; use `feature-flag-get-definition-by-key` to get it
from a key, or `feature-flag-get-all` to search by key or name.


Sets `active` to true and changes nothing else. This tool never accepts or replaces the `filters` object, so
targeting, rollout percentages, variants and payloads cannot change. There is no request body. To change who
the flag is served to, use `update-feature-flag`.


Returns 400 when the flag is archived (call `feature-flag-unarchive` first) or when a flag it depends on is
disabled. Returns 409 when an approval policy gates enabling. Enabling an already-enabled flag succeeds and
changes nothing, so retrying is safe.
Comment on lines +205 to +218

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Lifecycle tools accept soft-deleted flags

consider bug

Why we think it's a valid issue
  • Checked: the viewset's base queryset and where deleted is filtered, each lifecycle action's guard clauses, whether an equivalent guard already exists in this viewset, what evaluation does with a deleted flag, how an agent could obtain a deleted flag's ID, and the layer's test coverage.
  • Found: detail routes really do resolve soft-deleted flags. queryset = FeatureFlag.objects_including_soft_deleted.all() at products/feature_flags/backend/api/feature_flag.py:3129, with a comment at :3126-3128 stating that non-list actions can access soft-deleted flags, and only the list branch applies queryset.filter(deleted=False) at :3189-3191.
  • Found: none of the four actions check deleted. archive (:3591-3604), unarchive (:3620-3627) and _set_active (:3563-3575) go from self.get_object() straight to the facade. The viewset already owns a guard for exactly this — _deleted_flag_rejection at :3367-3379, applied at :3389 and :3425 — so the omission is inconsistent with the local pattern and the fix is two lines per action.
  • Found: the misleading-success consequence holds. FeatureFlagManager.get_queryset excludes deleted=True (products/feature_flags/backend/models/feature_flag.py:134-136), and the evaluation/cache paths read through FeatureFlag.objects (products/feature_flags/backend/flags_cache.py:916, products/feature_flags/backend/tasks.py:389), so a deleted flag never evaluates however active reads back. feature-flag-unarchive's promise to restore a flag "to the default flag list" (products/feature_flags/mcp/tools.yaml:266) is also false for a deleted flag, since the list filters it out.
  • Found: no coverage. products/feature_flags/backend/api/test/test_feature_flag_lifecycle_actions.py contains no deleted-flag case.
  • Impact: a caller holding a deleted flag's numeric ID gets 200 and active: true for a flag that serves nobody, or a reported restore that leaves the flag invisible. Nothing in the response or the description corrects that, which is why this stays valid rather than being dismissed as already-handled.
  • Priority: lowered to consider. The same behavior already exists on the tool these replace — partial_update (:3136-3138) adds no guard either, so update-feature-flag can enable a deleted flag on master today. Every lookup the new descriptions recommend is list-backed (feature-flag-get-all, and feature-flag-get-definition-by-key via the list endpoint), so both hide deleted flags and an agent has to be carrying a stale ID from an earlier turn or one the user supplied. Nothing is corrupted and the write is reversible, so the ceiling is a wrong report on a rare input rather than the should_fix impact claimed.
Issue description

The lifecycle endpoints can resolve soft-deleted flags and do not reject deleted: true. Enable can return active: true, but evaluation still excludes the flag. Unarchive can return success, but the default list still excludes it. A caller with an old numeric ID receives a successful result for an unusable flag.

Suggested fix

Reject deleted flags in every lifecycle endpoint. If restoration is supported, add a dedicated restore action and require it before enable or unarchive. Add tests for all four actions with a deleted flag.

Prompt to fix with AI (copy-paste)
## Context
@products/feature_flags/mcp/tools.yaml#L178-191
@products/feature_flags/mcp/tools.yaml#L264-275

<issue_description>
The lifecycle endpoints can resolve soft-deleted flags and do not reject `deleted: true`. Enable can return `active: true`, but evaluation still excludes the flag. Unarchive can return success, but the default list still excludes it. A caller with an old numeric ID receives a successful result for an unusable flag.
</issue_description>

<issue_validation>
- **Checked:** the viewset's base queryset and where `deleted` is filtered, each lifecycle action's guard clauses, whether an equivalent guard already exists in this viewset, what evaluation does with a deleted flag, how an agent could obtain a deleted flag's ID, and the layer's test coverage.
- **Found:** detail routes really do resolve soft-deleted flags. `queryset = FeatureFlag.objects_including_soft_deleted.all()` at `products/feature_flags/backend/api/feature_flag.py:3129`, with a comment at `:3126-3128` stating that non-list actions can access soft-deleted flags, and only the `list` branch applies `queryset.filter(deleted=False)` at `:3189-3191`.
- **Found:** none of the four actions check `deleted`. `archive` (`:3591-3604`), `unarchive` (`:3620-3627`) and `_set_active` (`:3563-3575`) go from `self.get_object()` straight to the facade. The viewset already owns a guard for exactly this — `_deleted_flag_rejection` at `:3367-3379`, applied at `:3389` and `:3425` — so the omission is inconsistent with the local pattern and the fix is two lines per action.
- **Found:** the misleading-success consequence holds. `FeatureFlagManager.get_queryset` excludes `deleted=True` (`products/feature_flags/backend/models/feature_flag.py:134-136`), and the evaluation/cache paths read through `FeatureFlag.objects` (`products/feature_flags/backend/flags_cache.py:916`, `products/feature_flags/backend/tasks.py:389`), so a deleted flag never evaluates however `active` reads back. `feature-flag-unarchive`'s promise to restore a flag "to the default flag list" (`products/feature_flags/mcp/tools.yaml:266`) is also false for a deleted flag, since the list filters it out.
- **Found:** no coverage. `products/feature_flags/backend/api/test/test_feature_flag_lifecycle_actions.py` contains no deleted-flag case.
- **Impact:** a caller holding a deleted flag's numeric ID gets 200 and `active: true` for a flag that serves nobody, or a reported restore that leaves the flag invisible. Nothing in the response or the description corrects that, which is why this stays valid rather than being dismissed as already-handled.
- **Priority:** lowered to `consider`. The same behavior already exists on the tool these replace — `partial_update` (`:3136-3138`) adds no guard either, so `update-feature-flag` can enable a deleted flag on master today. Every lookup the new descriptions recommend is list-backed (`feature-flag-get-all`, and `feature-flag-get-definition-by-key` via the list endpoint), so both hide deleted flags and an agent has to be carrying a stale ID from an earlier turn or one the user supplied. Nothing is corrupted and the write is reversible, so the ceiling is a wrong report on a rare input rather than the `should_fix` impact claimed.
</issue_validation>

## Task
Investigate the issue and solve it

<potential_solution>
Reject deleted flags in every lifecycle endpoint. If restoration is supported, add a dedicated restore action and require it before enable or unarchive. Add tests for all four actions with a deleted flag.
</potential_solution>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a real gap, but it belongs to the layer below, not this PR. The behavior the finding describes lives entirely in the lifecycle viewset actions (_set_active, archive, unarchive in products/feature_flags/backend/api/feature_flag.py): they resolve soft-deleted flags and don't reject deleted, unlike the sibling _deleted_flag_rejection guard already used by the usage-dashboard actions. That file ships in the base branch feat/flags-lifecycle-endpoints; this PR only wires the existing endpoints up as MCP tools and changes no backend code, so I can't fix it here without pulling a change into the wrong layer of the stack (the split was deliberate). It also needs a decision I shouldn't make unattended: reject deleted flags in all four actions, or add a dedicated restore action and require it before enable/unarchive — plus the backend tests for a deleted flag that are currently missing. Handing this to a human to land in the endpoints' own PR (or a follow-up) with the design choice made. I did not add a caveat to the tool descriptions instead: it wouldn't correct the misleading success response, and the recommended lookups (feature-flag-get-all, feature-flag-get-definition-by-key) are all list-backed and already hide deleted flags, so an agent can only reach this with a stale id carried from an earlier turn.



A key lookup that finds nothing does not mean the flag is absent. Archived flags are hidden from
`feature-flag-get-definition-by-key` and from `feature-flag-get-all`. Pass `{"archived":"true"}` to
`feature-flag-get-all` before you report the flag as missing.
Comment thread
posthog[bot] marked this conversation as resolved.
Resolved


To turn the flag on at a future time instead, use `scheduled-changes-create` with the `update_status`
operation.
param_overrides:
id:
cast: string-int
response:
include:
- id
- key
- active
- archived
- status
- version
feature-flag-get-all:
operation: feature_flags_list
enabled: true
Expand Down Expand Up @@ -153,7 +288,37 @@ tools:
cast: string-int
feature-flag-unarchive:
operation: feature_flags_unarchive_create
enabled: false
enabled: true
scopes:
- feature_flag:write
annotations:
readOnly: false
destructive: false
idempotent: true
enrich_url: '{id}'
title: Unarchive feature flag
description: >
Restore an archived feature flag to the default flag list. Requires the flag's numeric ID; pass
`{"archived":"true"}` to `feature-flag-get-all` to find archived flags.


Sets `archived` to false and changes nothing else. This tool never accepts or replaces the `filters` object.
There is no request body.


The flag stays turned off. Call `feature-flag-enable` when the user also wants it serving again. Unarchiving
a flag that is not archived succeeds and changes nothing, so retrying is safe.
param_overrides:
id:
cast: string-int
response:
include:
- id
- key
- active
- archived
- status
- version
feature-flags-activity-retrieve:
operation: feature_flags_activity_retrieve
enabled: true
Expand Down Expand Up @@ -499,12 +664,23 @@ tools:
enrich_url: '{id}'
title: Update feature flag
description: >
Update a feature flag by numeric ID (partial update — only the fields you include are changed). Call when
the user asks to enable or disable a flag, change its rollout percentage or release conditions, edit
variants or payloads, or update its description. Rollout conditions, variants, and payloads live in the
structured `filters` param, and sending `filters` replaces the whole object — read the flag first with
`feature-flag-get-definition` and merge your change. To apply a change at a future time instead, use
`scheduled-changes-create`. Use `feature-flag-get-all` to find the numeric ID from a key.
Update a feature flag by numeric ID (partial update: only the fields you include are changed). Use this for
edits that have no dedicated tool, such as release conditions, rollout percentages, variants, payloads, tags
and the description.


Prefer a dedicated tool whenever the request matches one. `feature-flag-enable`, `feature-flag-disable`,
`feature-flag-archive` and `feature-flag-unarchive` each change one state field and send no `filters`. A
targeting change someone else made between your read and your write is therefore not overwritten. Send
`active` or `archived` here only when the same request also changes targeting or another field, so the whole
edit lands in one write.


Rollout conditions, variants, and payloads live in the structured `filters` param, and sending `filters`
replaces the whole object, so read the flag first and merge your change. Use
`feature-flag-get-definition-by-key` when you have the string key: it returns the numeric ID and the full
definition in one call. Use `feature-flag-get-definition` when you already have the ID. To apply a change at
a future time instead, use `scheduled-changes-create`.
include_params:
- key
- name
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ Present the full cleanup prompt in a copyable format so the user can paste it di

Present the user with both options and their tradeoffs:

- **Disable** (`active: false`) via `posthog:update-feature-flag`: The flag stops being evaluated but the configuration is preserved. If something was missed in the code cleanup, re-enabling is instant. Recommended as the default.
- **Disable** via `posthog:feature-flag-disable`: The flag stops being evaluated but the configuration is preserved. The tool takes only the flag id, so targeting cannot change. If something was missed in the code cleanup, re-enabling is instant. Recommended as the default.
- **Delete** via `posthog:delete-feature-flag`: A soft-delete — the flag is marked as deleted but not physically removed. Keeps the flag list clean, but re-enabling requires recreating the flag. Better for flags the user is confident they'll never need again.

Once the user has chosen and confirms their code changes are deployed, apply the chosen action one flag at a time. Confirm each action so it's easy to stop if something goes wrong.
Expand Down Expand Up @@ -171,7 +171,7 @@ Agent steps:
- Delete: removes from the list, but you'd need to recreate if needed"

- User confirms: "Disable them, code is deployed"
- Disable each flag using posthog:update-feature-flag (active: false)
- Disable each flag using posthog:feature-flag-disable
- Confirm: "Both flags are now disabled in PostHog."
```

Expand All @@ -190,5 +190,5 @@ Agent steps:
- `posthog:feature-flag-get-all`: List and search feature flags (supports `active: "STALE"` filter)
- `posthog:feature-flag-get-definition`: Get full flag details including experiment associations
- `posthog:feature-flags-status-retrieve`: Get the status and reason for a single flag
- `posthog:update-feature-flag`: Disable a flag by setting `active: false`
- `posthog:feature-flag-disable`: Turn a flag off without touching its targeting
- `posthog:delete-feature-flag`: Soft-delete a flag
61 changes: 61 additions & 0 deletions services/mcp/evals/benchmark/tasks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,67 @@ tasks:
acceptable_tools: [feature-flag-get-all, feature-flag-get-definition]
success_criteria: "Resolves the flag by its key ('mcp-analytics') directly — does not guess a numeric id."

# The lifecycle tasks below all act on one flag key, so an agent-mode run flips only that
# flag's state. They score tool selection whether or not `mcp-eval-lifecycle` exists,
# because a missing flag must produce a clear answer rather than a fallback to the generic
# update tool. Nothing seeds the key today; seed it to also exercise the state change.
# One shared key makes the run order-dependent. `flag-unarchive-stays-off` needs the flag
# archived, and only `flag-archive-completed` leaves it that way. A second full run also
# starts from the targeting the first one wrote. Give each task its own key before relying
# on the state-change half of these scores.
- id: flag-enable-by-key
category: feature-flags
intent: 'Turn on the mcp-eval-lifecycle feature flag.'
expected_tools: [feature-flag-enable]
acceptable_tools: [feature-flag-get-definition-by-key, feature-flag-get-all, feature-flag-get-definition]
success_criteria: >
Calls feature-flag-enable rather than a generic update, and sends no filters payload in any call. If the
flag does not exist, says so rather than reaching for update-feature-flag.

- id: flag-disable-reports-impact
category: feature-flags
intent: "We're seeing errors from the mcp-eval-lifecycle flag. Switch it off."
expected_tools: [feature-flag-disable]
acceptable_tools:
[
feature-flag-get-definition-by-key,
feature-flag-get-all,
feature-flag-get-definition,
feature-flags-dependent-flags-retrieve,
]
success_criteria: >
Resolves the key to an id and calls feature-flag-disable with no filters payload. Reads the definition first
and says what else the flag is wired into (linked experiments, surveys, replay settings), or that nothing
is. If the flag does not exist, says so rather than reaching for update-feature-flag.

- id: flag-archive-completed
category: feature-flags
intent: 'The mcp-eval-lifecycle rollout is finished. Archive the flag so it stops cluttering the list.'
expected_tools: [feature-flag-archive]
acceptable_tools: [feature-flag-get-definition-by-key, feature-flag-get-all, feature-flag-get-definition]
success_criteria: >
Calls feature-flag-archive with no filters payload, and tells the user archiving also turns the flag off.
If the flag does not exist, says so rather than reaching for update-feature-flag.

- id: flag-unarchive-stays-off
category: feature-flags
intent: 'Bring the mcp-eval-lifecycle flag back into the flag list, I need to look at it again.'
expected_tools: [feature-flag-unarchive]
acceptable_tools: [feature-flag-get-all, feature-flag-get-definition-by-key, feature-flag-get-definition]
success_criteria: >
Calls feature-flag-unarchive with no filters payload and does not enable it, saying the flag is still turned
off. Finds the archived flag by passing {"archived":"true"} to feature-flag-get-all; a plain key lookup returning
nothing is not grounds for reporting that the flag does not exist.

- id: flag-targeting-edit-uses-update
category: feature-flags
intent: 'Change the mcp-eval-lifecycle flag so it only applies to users on the @example.com domain.'
expected_tools: [update-feature-flag]
acceptable_tools: [feature-flag-get-definition-by-key, feature-flag-get-definition, feature-flag-get-all]
success_criteria: >
Reads the current definition first, then edits targeting through update-feature-flag. Does not reach for a
lifecycle tool, which cannot change targeting. If the flag does not exist, says so.

# --- session-replay ----------------------------------------------------------
- id: replay-rage-clicks
category: session-replay
Expand Down
Loading
Loading