chore(feature_flags): rename bulk_keys MCP op so the name matches its read-only behavior#60741
Conversation
MCP UI Apps size report
|
|
⏭️ Skipped snapshot commit because branch advanced to The new commit will trigger its own snapshot update workflow. If you expected this workflow to succeed: This can happen due to concurrent commits. To get a fresh workflow run, either:
|
|
Size Change: 0 B Total Size: 80.9 MB ℹ️ View Unchanged
|
|
🎭 Playwright report · View test results →
These issues are not necessarily caused by your changes. |
|
Reviews (1): Last reviewed commit: "chore: update OpenAPI generated types" | Re-trigger Greptile |
There was a problem hiding this comment.
The rename is applied consistently across all 9 files and the logic is sound (removing a read-only op from a write-tools forbidden list). However, this is a breaking API contract change — the MCP tool name feature-flags-bulk-keys-create is renamed to feature-flags-bulk-keys-retrieve, which will silently break any deployed agent or client that references the old name. With zero reviews on a cross-cutting change touching MCP contracts, OpenAPI operation IDs, and security eval logic, a human reviewer should confirm there are no active consumers of the old tool name before this merges.
… read-only behavior The bulk_keys action on the feature flag viewset is a read-only lookup (input: list of IDs, output: ID -> key mapping, scope: feature_flag:read). drf-spectacular's default operation_id naming convention for POST extra actions produced "feature_flags_bulk_keys_create", which then flowed downstream as the MCP tool name "feature-flags-bulk-keys-create" - a "create"-flavored name on a tool whose readOnly: true annotation is correct. The mismatch was also misread as a write capability and added to the survey agent's forbidden-write tools list. Override the operation_id to "feature_flags_bulk_keys_retrieve" so the generated tool name reads as a lookup, drop the entry from the survey forbidden-writes set, and propagate the rename through the generated OpenAPI/MCP artifacts. The annotation itself stays readOnly: true, which it already was. Also rename the MCP tool-schema snapshot fixture to match the new runtime tool name so the MCP unit tests pass. Generated-By: PostHog Code Task-Id: b57e8991-0290-4544-8a19-d1be9d30d3ab
bba947b to
c2719a9
Compare
Merge activity
|
… read-only behavior (#60741) ## Problem The MCP tool `feature-flags-bulk-keys-create` looks like a write operation but isn't — the underlying `bulk_keys` action on the feature-flag viewset only resolves a list of flag IDs to their string keys (`feature_flag:read` scope, no mutation). The `_create` suffix came from drf-spectacular's default operation_id convention for POST extra-actions, then flowed downstream into the MCP tool name unchanged. This came up in a review of PostHog's ChatGPT integration: the tool's `readOnly: true` MCP annotation is correct, but the name actively contradicts it, which is confusing both to MCP clients and (it turns out) to the survey agent's eval scaffolding, where the tool had been added to `SURVEY_FORBIDDEN_WRITE_TOOLS` — i.e. a read-only tool was being blocked as if it were a write. POST (vs. GET) is still the right HTTP verb here because the input is a potentially-long list of IDs that doesn't belong in a query string; this PR doesn't change the endpoint, just the name MCP/codegen derives from it. ## Changes - `products/feature_flags/backend/api/feature_flag.py`: override `operation_id="feature_flags_bulk_keys_retrieve"` on `@extend_schema` so drf-spectacular stops appending `_create`. Same pattern already used by `feature_flags_all_activity_retrieve` a few hundred lines below. - `products/feature_flags/mcp/tools.yaml`: rename the tool key + operation accordingly. `readOnly: true` / `feature_flag:read` are unchanged — they were already correct. - `ee/hogai/eval/sandboxed/surveys/scorers.py`: drop the entry from `SURVEY_FORBIDDEN_WRITE_TOOLS` — it's a read tool, doesn't belong there. - Generated artifacts (`services/mcp/**`, `products/feature_flags/frontend/generated/**`): propagate the rename. These are mechanical (`*BulkKeysCreate*` → `*BulkKeysRetrieve*` everywhere); see "Agent context" below. Tool name goes from `feature-flags-bulk-keys-create` → `feature-flags-bulk-keys-retrieve`. The HTTP route, request/response shape, and scopes are untouched. ## How did you test this code? I'm an agent and did not run the test suite or `hogli build:openapi` (the sandbox doesn't have a Postgres for the schema-gen step). Verified by: - grep for `BulkKeysCreate` / `bulk-keys-create` / `bulk_keys_create` across the tree returns zero hits after the rename. - Generated-file rename is consistent (imports, schema vars, handler, TOOL_MAP key, both schema JSONs, frontend `api.ts` / `api.zod.ts`). - Cross-checked the new operation_id suffix `_retrieve` against the existing `feature_flags_all_activity_retrieve` precedent in the same viewset. Please regenerate locally with `hogli build:openapi` before merge to confirm the manual edits match the generator's output byte-for-byte — if there's a drift I missed, that's the place it'll show up. ## Automatic notifications - [ ] Publish to changelog? - [ ] Alert Sales and Marketing teams? ## Docs update No docs to update — this is an internal MCP/codegen rename, not a public API change. ## 🤖 Agent context Authored by an agent (Claude) following a Slack thread where @rafael flagged the `readOnly` annotation looking wrong on `feature-flags-bulk-keys-create` while reviewing the ChatGPT app's tool surface. Initial read was that `readOnly: true` should flip to `false` (since the name says "create"); on inspection the endpoint is genuinely read-only and the annotation is correct — the name is the lie. Rafa confirmed the fix should be "make the name tell the truth" instead. Tradeoff considered: could've renamed the DRF action method (`bulk_keys` → `bulk_keys_lookup` or similar), which would change the URL too — rejected as a breaking API change for any existing callers of `/api/projects/{project_id}/feature_flags/bulk_keys/`. Overriding `operation_id` in the schema decorator is the smallest blast radius. Generated files were updated by hand because `hogli build:openapi` requires a Postgres connection that wasn't available in the sandbox — flagged in the test plan above.
Problem
The MCP tool
feature-flags-bulk-keys-createlooks like a write operation but isn't — the underlyingbulk_keysaction on the feature-flag viewset only resolves a list of flag IDs to their string keys (feature_flag:readscope, no mutation). The_createsuffix came from drf-spectacular's default operation_id convention for POST extra-actions, then flowed downstream into the MCP tool name unchanged.This came up in a review of PostHog's ChatGPT integration: the tool's
readOnly: trueMCP annotation is correct, but the name actively contradicts it, which is confusing both to MCP clients and (it turns out) to the survey agent's eval scaffolding, where the tool had been added toSURVEY_FORBIDDEN_WRITE_TOOLS— i.e. a read-only tool was being blocked as if it were a write.POST (vs. GET) is still the right HTTP verb here because the input is a potentially-long list of IDs that doesn't belong in a query string; this PR doesn't change the endpoint, just the name MCP/codegen derives from it.
Changes
products/feature_flags/backend/api/feature_flag.py: overrideoperation_id="feature_flags_bulk_keys_retrieve"on@extend_schemaso drf-spectacular stops appending_create. Same pattern already used byfeature_flags_all_activity_retrievea few hundred lines below.products/feature_flags/mcp/tools.yaml: rename the tool key + operation accordingly.readOnly: true/feature_flag:readare unchanged — they were already correct.ee/hogai/eval/sandboxed/surveys/scorers.py: drop the entry fromSURVEY_FORBIDDEN_WRITE_TOOLS— it's a read tool, doesn't belong there.services/mcp/**,products/feature_flags/frontend/generated/**): propagate the rename. These are mechanical (*BulkKeysCreate*→*BulkKeysRetrieve*everywhere); see "Agent context" below.Tool name goes from
feature-flags-bulk-keys-create→feature-flags-bulk-keys-retrieve. The HTTP route, request/response shape, and scopes are untouched.How did you test this code?
I'm an agent and did not run the test suite or
hogli build:openapi(the sandbox doesn't have a Postgres for the schema-gen step). Verified by:BulkKeysCreate/bulk-keys-create/bulk_keys_createacross the tree returns zero hits after the rename.api.ts/api.zod.ts)._retrieveagainst the existingfeature_flags_all_activity_retrieveprecedent in the same viewset.Please regenerate locally with
hogli build:openapibefore merge to confirm the manual edits match the generator's output byte-for-byte — if there's a drift I missed, that's the place it'll show up.Automatic notifications
Docs update
No docs to update — this is an internal MCP/codegen rename, not a public API change.
🤖 Agent context
Authored by an agent (Claude) following a Slack thread where @rafael flagged the
readOnlyannotation looking wrong onfeature-flags-bulk-keys-createwhile reviewing the ChatGPT app's tool surface. Initial read was thatreadOnly: trueshould flip tofalse(since the name says "create"); on inspection the endpoint is genuinely read-only and the annotation is correct — the name is the lie. Rafa confirmed the fix should be "make the name tell the truth" instead.Tradeoff considered: could've renamed the DRF action method (
bulk_keys→bulk_keys_lookupor similar), which would change the URL too — rejected as a breaking API change for any existing callers of/api/projects/{project_id}/feature_flags/bulk_keys/. Overridingoperation_idin the schema decorator is the smallest blast radius.Generated files were updated by hand because
hogli build:openapirequires a Postgres connection that wasn't available in the sandbox — flagged in the test plan above.