Skip to content

feat(mcp): report the release-condition change from update-feature-flag - #114208

Open
jakesciotto wants to merge 1 commit into
posthog/mcp-update-feature-flag-add-users-descriptionfrom
posthog/mcp-update-feature-flag-filters-change
Open

jakesciotto wants to merge 1 commit into
posthog/mcp-update-feature-flag-add-users-descriptionfrom
posthog/mcp-update-feature-flag-filters-change

Conversation

@jakesciotto

Copy link
Copy Markdown
Contributor

Problem

An agent asked through MCP to add people to a feature flag can read the request as "restrict the flag to these people", add an is_not property filter to an existing release condition, and drop users who had the flag. The update-feature-flag result today is only the updated flag, so the agent gets no signal that its write removed access before it reports success.

This is the second layer of a stack. The layer below, #114186, tells the agent in the tool description which edits add users and which edits remove them.

Changes

  • When filters is sent, the tool result gains filters_change: changed, narrows, conditions_added, conditions_removed, conditions_changed (added, removed and changed property filters, plus the rollout before and after) and a summary in plain words with conditions numbered from 1 as in the UI. Example summary: "Condition 1 gained the filter organization_id is_not [...], so it now serves fewer users."
  • narrows is true when a changed condition gained a property filter, an exact filter lost values, an is_not filter gained values, a condition was removed, or a rollout percentage went down.
  • The description ends with a new paragraph: "When you send filters, the response includes filters_change. Read its summary. When filters_change.narrows is true, tell the user which condition now serves fewer users, and confirm that they asked to restrict access."
  • The request hook keeps the existing flag's filters from the GET it already makes, and the response hook compares them with the filters in the PATCH response. No API request is added and the PATCH body is unchanged.
  • Mechanical: services/mcp/schema/generated-tool-definitions.json and services/mcp/schema/tool-definitions-all.json regenerated; only the description string changed.

Note

Conditions are matched by property set, variant and group aggregation, so reordering conditions, or the values inside a filter, reports no change. Conditions left unmatched pair up in order and report as changed, so a rewritten condition reads as one change rather than a removal plus an addition.

How did you test this code?

Test rationale: The regression is the tool result omitting or misreporting the effect of a filters write. The closest existing test, update-feature-flag-preserving-groups.test.ts, checks the request side (the merge and the PATCH body) and asserts nothing about the result, so the new cases live in a sibling unit test that drives the same generated handler with a mocked API.

New unit tests in services/mcp/tests/unit/update-feature-flag-filters-change.test.ts:

  1. An is_not filter added to an existing condition: narrows is true and the filter is in properties_added.
  2. Two values added to an existing exact filter: narrows is false and values_added lists both.
  3. A condition inserted at index 0 with the old ones kept: conditions_added is [0], conditions_changed is empty, narrows is false.
  4. The same conditions in a different order: changed is false.
  5. No filters param: no filters_change key and one API request, the PATCH.

Commands run locally:

  • pnpm --filter=@posthog/mcp exec vitest run tests/unit
  • pnpm --filter=@posthog/mcp run typecheck
  • pnpm --filter=@posthog/mcp run lint
  • pnpm --filter=@posthog/mcp run format:check reports three files this PR does not touch (src/tools/notebooks/runNotebook.ts, src/tools/notebooks/runNotebookStatus.ts, src/tools/render-ui.ts); the files in this PR pass.
  • pnpm --filter=@posthog/mcp run generate-tools and pnpm --filter=@posthog/mcp run lint-tool-names

No live MCP session ran against a PostHog instance, and no agent eval ran, so the change in agent behavior is not measured.

Release status

  • No feature flag controls this change

Docs update

None. The tool description is the documentation surface for this tool.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Claude Code, Claude Fable 5.1

Written from a work order in a PostHog Desktop task. The motivating case was a customer report of an agent narrowing a flag; nothing from that report is in this PR, and the test data is invented. Stacked on #114186 because both PRs edit the same description and the same generated files. Repo skills read before writing: writing-pr-descriptions, writing-tests, stacking-prs, implementing-mcp-tools. The summary wording was checked by hand against extra scenarios (rollout changes, removed conditions, scalar value changes) beyond the five committed cases.


Created with PostHog Desktop

🤖 Generated with Claude Code

When update-feature-flag writes filters, the response now carries filters_change: which release conditions were added, removed or changed, whether the write narrows access (narrows), and a plain-language summary. The request hook keeps the existing flag's filters from the GET it already makes, so no API request is added. The tool description tells agents to read the summary and confirm with the user when narrows is true.

Generated-By: PostHog Desktop
Task-Id: f04f7fea-6193-47e7-ba30-c9e5cdb94431
@jakesciotto jakesciotto self-assigned this Oct 8, 2026
@jakesciotto
jakesciotto added this pull request to stack #114209 October 8, 2026 22:01
@github-actions

github-actions Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

⚠️ Trunk lane — backend Python lane

This PR is assigned to the backend Python lane. It runs backend Python tests and may merge in parallel with PRs in other lanes.

✅ Duplication (Python) — clean

New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

✅ Duplication (TypeScript) — clean

New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

✅ Bundle size — no change

Uncompressed size of every built .js bundle, compared against the base branch.

Total: 76.77 MiB · no change

No file changed by more than 1000 B.

Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report

✅ Eager graph — within budget

How much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy import() / React.lazy chunks are not counted.

Root Eager (shipped) Δ vs base Budget
entry (logged-out pages, app bootstrap)
src/index.tsx
1.65 MiB · 23 files no change █████████░ 89.8% of 1.84 MiB
logged-out boot: index + App + bootApp (preloaded by every page, including /login)
src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
3.80 MiB · 675 files no change █████████░ 94.3% of 4.03 MiB
authenticated shell (every logged-in page)
src/scenes/AuthenticatedShell.tsx
7.74 MiB · 2,456 files no change █████████░ 92.8% of 8.34 MiB
dashboard scene
src/scenes/dashboard/Dashboard.tsx
9.97 MiB · 3,538 files no change █████████░ 91.3% of 10.92 MiB
today home path
src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
7.76 MiB · 2,466 files no change █████████░ 90.4% of 8.58 MiB
events scene
src/scenes/activity/explore/EventsScene.tsx
9.59 MiB · 3,389 files no change █████████░ 91.3% of 10.51 MiB
replay detail scene
src/scenes/session-recordings/detail/SessionRecordingDetail.tsx
12.29 MiB · 4,185 files no change ████████░░ 78.2% of 15.72 MiB

🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 node_modules/@posthog/brand/dist/generated/hoggies/svg/ stays out of src/index.tsx
🟢 node_modules/@posthog/brand/dist/generated/hoggies/components/ stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/layout/navigation-3000/navigationLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/scenes/dashboard/dashboardLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/lemon-ui/LemonMarkdown/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/RichContentEditor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/CodeSnippet/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/taxonomy/core-filter-definitions-by-group.json stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 products/dashboards/frontend/widgets/previews/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts stays out of src/scenes/AuthenticatedShell.tsx
🟢 zod/v4/locales/de.js stays out of src/scenes/AuthenticatedShell.tsx
🟢 node_modules/@posthog/brand/dist/generated/hoggies/svg/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 node_modules/@posthog/brand/dist/generated/hoggies/components/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/scenes/session-recordings/playlist/SessionRecordingsPlaylist.tsx stays out of src/scenes/dashboard/Dashboard.tsx
🟢 src/scenes/web-analytics/tiles/WebAnalyticsTile.tsx stays out of src/scenes/dashboard/Dashboard.tsx
🟢 src/scenes/project-homepage/ai-first/AiFirstHomepage.tsx stays out of src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
🟢 src/scenes/project-homepage/today/TodayReportPage.tsx stays out of src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
🟢 src/queries/Query/Query.tsx stays out of src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
🟢 src/scenes/session-recordings/playlist/SessionRecordingsPlaylist.tsx stays out of src/scenes/activity/explore/EventsScene.tsx
🟢 src/scenes/web-analytics/tiles/WebAnalyticsTile.tsx stays out of src/scenes/activity/explore/EventsScene.tsx

Largest files eagerly shipped from src/index.tsx
Size File
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
24.6 KiB ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js
6.3 KiB ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js
4.5 KiB ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js
3.9 KiB ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js
1.4 KiB ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js
1.3 KiB src/index.tsx
1.3 KiB src/RootErrorBoundary.tsx
912 B ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js
839 B src/scenes/ChunkLoadErrorBoundary.tsx
Largest files eagerly shipped from src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
Size File
316.7 KiB ../node_modules/.pnpm/posthog-js@1.438.1_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
220.8 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
99.8 KiB src/lib/api.ts
93.1 KiB src/products.tsx
69.0 KiB src/lib/lemon-ui/icons/icons.tsx
40.7 KiB src/lib/utils/eventUsageLogic.ts
38.7 KiB ../node_modules/.pnpm/@dnd-kit+core@6.0.8_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@dnd-kit/core/dist/core.esm.js
33.9 KiB ../node_modules/.pnpm/kea@4.0.0-pre.6_patch_hash=139b8d1f1304f9d9da452a9a1244c94ea679dbcb85687d8999563146879fb6f5_react@18.3.1/node_modules/kea/lib/index.cjs.js
29.0 KiB ../node_modules/.pnpm/zod@4.3.6/node_modules/zod/v4/core/schemas.js
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
Size File
316.7 KiB ../node_modules/.pnpm/posthog-js@1.438.1_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
281.0 KiB src/taxonomy/core-filter-definitions-by-group.json
220.8 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
153.7 KiB ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
112.1 KiB ../packages/quill/packages/quill/dist/index.js
99.8 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
93.1 KiB src/products.tsx
90.6 KiB ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js
Largest files eagerly shipped from src/scenes/dashboard/Dashboard.tsx
Size File
316.7 KiB ../node_modules/.pnpm/posthog-js@1.438.1_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
281.0 KiB src/taxonomy/core-filter-definitions-by-group.json
220.8 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
181.9 KiB src/queries/validators.js
153.7 KiB ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
112.1 KiB ../packages/quill/packages/quill/dist/index.js
99.8 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
93.1 KiB src/products.tsx
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
Size File
316.7 KiB ../node_modules/.pnpm/posthog-js@1.438.1_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
281.0 KiB src/taxonomy/core-filter-definitions-by-group.json
220.8 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
153.7 KiB ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
112.1 KiB ../packages/quill/packages/quill/dist/index.js
99.8 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
93.1 KiB src/products.tsx
90.6 KiB ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js
Largest files eagerly shipped from src/scenes/activity/explore/EventsScene.tsx
Size File
316.7 KiB ../node_modules/.pnpm/posthog-js@1.438.1_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
281.0 KiB src/taxonomy/core-filter-definitions-by-group.json
220.8 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
181.9 KiB src/queries/validators.js
153.7 KiB ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
112.1 KiB ../packages/quill/packages/quill/dist/index.js
99.8 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
93.1 KiB src/products.tsx
Largest files eagerly shipped from src/scenes/session-recordings/detail/SessionRecordingDetail.tsx
Size File
316.7 KiB ../node_modules/.pnpm/posthog-js@1.438.1_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
316.0 KiB ../node_modules/.pnpm/posthog-js@1.438.1_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/rrweb.js
281.0 KiB src/taxonomy/core-filter-definitions-by-group.json
220.8 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
181.9 KiB src/queries/validators.js
153.7 KiB ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
112.1 KiB ../packages/quill/packages/quill/dist/index.js
99.8 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js

Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479

✅ Toolbar bundle — eager 2.24 MiB within budget

What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.

Metric Size Δ vs base Budget
Eager (shipped)
entry + static imports
2.24 MiB · 19 files no change ████░░░░░░ 39.1% of 5.72 MiB
Deferred (lazy) 2.18 MiB · 44 files no change n/a — loads on demand
Loader dist/toolbar.js 1.2 KiB no change █░░░░░░░░░ 6.0% of 19.5 KiB
Largest eagerly-shipped chunks
Size File
861.3 KiB dist/toolbar/toolbar-app-G6HPIREX.css
669.7 KiB dist/toolbar/chunk-chunk-RW27W6OZ.js
259.4 KiB dist/toolbar/chunk-chunk-YKNMISGG.js
138.2 KiB dist/toolbar/chunk-chunk-AGSLNSZV.js
131.8 KiB dist/toolbar/chunk-chunk-FDH2IBXT.js
75.2 KiB dist/toolbar/toolbar-app-6OWAMBXW.js
69.0 KiB dist/toolbar/chunk-chunk-TSAL54PB.js
35.6 KiB dist/toolbar/chunk-chunk-MZXZUNS3.js
21.7 KiB dist/toolbar/chunk-chunk-YLRUJB4T.js
6.8 KiB dist/toolbar/chunk-chunk-DV7IWQNF.js

Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile

✅ Dist folder size — no change

Total size of the built frontend/dist folder (all assets), compared against the base branch.

Total: 1010.23 MiB · no change

ℹ️ MCP UI apps size — 32 app(s), 17186.0 KB JS

Built size of each MCP UI app (main.js + styles.css).

App JS CSS
debug 597.9 KB 203.4 KB
action 454.2 KB 203.4 KB
action-list 564.3 KB 203.4 KB
cohort 453.2 KB 203.4 KB
cohort-list 563.3 KB 203.4 KB
email-template 453.0 KB 203.4 KB
error-details 469.7 KB 203.4 KB
error-issue 454.6 KB 203.4 KB
error-issue-list 564.9 KB 203.4 KB
experiment 561.4 KB 203.4 KB
experiment-list 565.0 KB 203.4 KB
experiment-results 566.4 KB 203.4 KB
feature-flag 566.9 KB 203.4 KB
feature-flag-list 570.6 KB 203.4 KB
feature-flag-testing 457.4 KB 203.4 KB
inline-scan 453.7 KB 203.4 KB
insight-actors 562.4 KB 203.4 KB
llm-costs 559.4 KB 203.4 KB
session-recording 455.4 KB 203.4 KB
survey 454.8 KB 203.4 KB
survey-global-stats 562.0 KB 203.4 KB
survey-list 565.0 KB 203.4 KB
survey-stats 562.0 KB 203.4 KB
trace-span 453.6 KB 203.4 KB
trace-span-list 564.2 KB 203.4 KB
vision-observation-list 563.4 KB 203.4 KB
workflow 453.5 KB 203.4 KB
workflow-list 563.6 KB 203.4 KB
loops-review 457.9 KB 203.4 KB
query-results 775.6 KB 203.4 KB
render-ui 859.0 KB 203.4 KB
visual-review-snapshots 458.0 KB 203.4 KB
ℹ️ MCP agent API — agent-facing tool changes

What this PR changes for agents, from the tool schema snapshots and tool definitions.

Tools changed (1):

Tool Params Scopes Annotations Schema chars
update-feature-flag description changed

@greptile-apps

greptile-apps Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Retrigger

[Medium impact] Adds change reporting to feature flag updates.

Fix the missed scalar restriction warning and satisfy the test-mock requirement before merging.

Reviews (1) · Last reviewed commit: "feat(mcp): report the release-condition ..." · Reviewed by Greptile

Comment on lines +196 to +203
function propertyChangeNarrows(change: PropertyChange): boolean {
if (change.operator === 'exact') {
return (change.values_removed?.length ?? 0) > 0
}
if (change.operator === 'is_not') {
return (change.values_added?.length ?? 0) > 0
}
return false

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.

P1 Scalar edits miss restriction warnings

Changing a scalar exact filter from "enterprise" to "pro" removes access from enterprise users, but propertyChangeNarrows returns false. Scalar changes populate value_before and value_after, while this check only reads array changes. Both the tool schema and flag evaluator support scalar values, so an agent can miss the required restriction warning.

Compare scalar values as single-value sets for exact and is_not, including scalar-to-array changes, and test those paths.

Prompt To Fix With AI
This is a comment left during a code review.
Path: services/mcp/src/tools/featureFlags/describeFiltersChange.ts
Line: 196-203

Comment:
**Scalar edits miss restriction warnings**

Changing a scalar `exact` filter from `"enterprise"` to `"pro"` removes access from enterprise users, but `propertyChangeNarrows` returns false. Scalar changes populate `value_before` and `value_after`, while this check only reads array changes. Both the tool schema and flag evaluator support scalar values, so an agent can miss the required restriction warning.

Compare scalar values as single-value sets for `exact` and `is_not`, including scalar-to-array changes, and test those paths.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Comment on lines +304 to +307
if (conditionNarrows(entry.change)) {
effect = ', so it now serves fewer users'
} else if (conditionWidens(entry.change)) {
effect = ', so it now serves more users'

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.

P2 Mixed edits misreport lost access

The summary says a condition serves fewer users whenever one part of the edit narrows it, even if another part expands it. Adding a property filter while increasing rollout from 0% to 100% reports “fewer users,” although the condition previously served nobody. An agent can pass that incorrect claim to the user.

Keep the conservative narrows signal, but describe mixed changes without claiming a definite increase or decrease.

Prompt To Fix With AI
This is a comment left during a code review.
Path: services/mcp/src/tools/featureFlags/describeFiltersChange.ts
Line: 304-307

Comment:
**Mixed edits misreport lost access**

The summary says a condition serves fewer users whenever one part of the edit narrows it, even if another part expands it. Adding a property filter while increasing rollout from 0% to 100% reports “fewer users,” although the condition previously served nobody. An agent can pass that incorrect claim to the user.

Keep the conservative `narrows` signal, but describe mixed changes without claiming a definite increase or decrease.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

if (sentences.length <= 3) {
return sentences.join(' ')
}
return `${sentences.slice(0, 2).join(' ')} ${sentences.length - 2} more changes are listed in conditions_added, conditions_removed and conditions_changed.`

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.

P2 Summary hides restricted conditions

When there are four or more summary sentences, only the first two survive. An edit that expands conditions 1–3 but restricts condition 4 returns narrows: true with a summary that names only expansions. The tool description tells agents to read that summary and identify the restricted condition, so larger edits can hide the detail they need.

Preserve narrowing details when shortening the summary, or explicitly list every restricted condition.

Prompt To Fix With AI
This is a comment left during a code review.
Path: services/mcp/src/tools/featureFlags/describeFiltersChange.ts
Line: 333

Comment:
**Summary hides restricted conditions**

When there are four or more summary sentences, only the first two survive. An edit that expands conditions 1–3 but restricts condition 4 returns `narrows: true` with a summary that names only expansions. The tool description tells agents to read that summary and identify the restricted condition, so larger edits can hide the detail they need.

Preserve narrowing details when shortening the summary, or explicitly list every restricted condition.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Comment on lines +26 to +30
} as any,
stateManager: { getProjectId: vi.fn().mockResolvedValue('42') } as any,
env: {} as any,
sessionManager: {} as any,
cache: {} as any,

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.

P2 Mock casts hide missing methods

createMockContext casts incomplete dependencies to any, bypassing checks that would catch missing methods when Context changes. The repository requires test mocks to include required interface properties rather than cast them away.

Use typed mock builders that satisfy those interfaces. This repository requirement must be satisfied before merging.

Rule Used: When creating mock objects for tests, include all required properties from the interface to avoid casting and potential issues in the future. (source)

Learned From
PostHog/posthog#32521

Prompt To Fix With AI
This is a comment left during a code review.
Path: services/mcp/tests/unit/update-feature-flag-filters-change.test.ts
Line: 26-30

Comment:
**Mock casts hide missing methods**

`createMockContext` casts incomplete dependencies to `any`, bypassing checks that would catch missing methods when `Context` changes. The repository requires test mocks to include required interface properties rather than cast them away.

Use typed mock builders that satisfy those interfaces. This repository requirement must be satisfied before merging.

**Rule Used:** When creating mock objects for tests, include all required properties from the interface to avoid casting and potential issues in the future. ([source](https://app.greptile.com/posthog-org-19734/-/custom-context?memory=693b8339-f300-4cf5-bacb-3f1630384594))

**Learned From**
[PostHog/posthog#32521](https://github.com/PostHog/posthog/pull/32521)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@jakesciotto
jakesciotto marked this pull request as ready for review October 8, 2026 22:11
@parameterai

parameterai Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Risk: Medium · 1 medium, 1 low

This PR adds a filters_change report to the update-feature-flag MCP tool: the response hook diffs the flag's release conditions before and after a filters write and tells the agent whether the edit narrowed access. The diff logic itself is the main risk: it does unbounded quadratic matching synchronously on the shared hosted MCP event loop (one crafted call can stall other users), and the narrows verdict misses narrowing edits under every operator except exact/is_not, silently defeating the new safety warning for those cases.

Sentinel reviewed 6e4b82c · Review settings

@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team October 8, 2026 22:11
@pr-assigner-resolver-posthog

Copy link
Copy Markdown

👀 Auto-assigned reviewers

These soft owners were skipped because they only have minor changes here. Nothing blocks merge, so self-assign if you'd like a look:

  • @PostHog/team-feature-flags (products/feature_flags/product.yaml)

Soft owners come from each directory's owners.yaml and each product's product.yaml (resolved nearest-file-wins). For a skipped owner, the locator is the file that decided it. Generated files and lockfiles are ignored when deciding ownership.

const unmatchedAfter = new Set(after)
const pairs: Array<[T, T]> = []
for (const isSame of passes) {
for (const next of after) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 One crafted update-feature-flag call can block the shared hosted MCP server for minutes

The new filters_change diff runs matchInOrder synchronously on the hosted multi-user MCP server (mcp.posthog.com), with work quadratic in the condition count and no bound on either side. A single tool call can pin the Node event loop for minutes, stalling every other user's request.

How:

  1. Create a feature flag with ~100k release conditions via the public PostHog API directly (no MCP body limit applies there, and no group-count bound exists in filters_validation.py).
  2. Call update-feature-flag through the hosted MCP with ~15k distinct conditions — within the 1 MiB body limit (services/mcp/src/hono/dispatcher.ts:55).
  3. The request hook GETs the huge existing flag, and the PATCH returns the new one; afterResponse then runs describeFiltersChange.
  4. The nested loop at describeFiltersChange.ts:123-127 does passes(2) × |after| × |before| ≈ 3×10⁹ Set lookups/signature compares, fully synchronous — tool-executor.ts awaits tool.handler directly on the event loop with no timeout, and no timeout can preempt synchronous code.

Fix: Cap the diff (e.g., skip filters_change and say "too many conditions to summarize" above a few hundred conditions per side) before running the matching.


React with 👍 if useful or 👎 if not

}
}

function propertyChangeNarrows(change: PropertyChange): boolean {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Narrowing edits under operators other than exact/is_not are reported as not narrowing

The narrows safety signal this PR adds silently returns false for whole classes of audience-narrowing edits, so an agent removes users' access and reports success with no warning — exactly the failure this change exists to prevent.

How:

  1. A user asks an agent to remove two email domains from a flag whose condition filters email icontains_multi ["enterprise.com", "pro.com"] (schema forces array values for that operator).
  2. The agent sends filters with value: ["enterprise.com"].
  3. propertyChangeNarrows at describeFiltersChange.ts:196-203 only handles 'exact' and 'is_not'; for icontains_multi it returns false, and values_removed narrowing under starts_with, gt/lt, regex, or a scalar exact value change is likewise ignored.
  4. filters_change.narrows is false, so per the new tool description the agent tells the user nothing while the write removed access.

Fix: Treat values_removed on any array-valued operator as narrowing (or explicitly list narrowing semantics per operator) in propertyChangeNarrows.


React with 👍 if useful or 👎 if not

@trunk-io

trunk-io Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Static Badge   Static Badge   Static Badge

View Full Report ↗︎ ⋅ Docs

This branch has not been deployed

No deployments
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