Skip to content

feat: wire native FME feature flag definition list, delete, and execute - #831

Open
ravindraharness wants to merge 3 commits into
harness:mainfrom
ravindraharness:fme-v4-remaining-flag-definition
Open

feat: wire native FME feature flag definition list, delete, and execute#831
ravindraharness wants to merge 3 commits into
harness:mainfrom
ravindraharness:fme-v4-remaining-flag-definition

Conversation

@ravindraharness

@ravindraharness ravindraharness commented Aug 17, 2026

Copy link
Copy Markdown

Description

Follow-up to #806 for remaining FME feature flag definition operations, plus the leftover native flag create input aliases from #830 (that PR is closed as a duplicate of this delta).

#806 dual-moded get / create / update (workspace_id or org_id+project_id). MCP never had list, delete, kill, restore, or reallocate on workspace_id for this resource, so those five operations are Harness-native only. This PR does not invent a legacy Split v2 MCP contract for them.

Definition get/create/update are unchanged from #806 (legacy still uses /internal/api/v2/splits/...; native still uses /fme/api/v4/feature-flag-definitions with PATCH + JSON Merge Patch on update).

Dual-mode routing — this PR only

Legend: wired · native-only (rejects workspace_id) · unchanged from #806

Resource Legacy (workspace_id) Harness-native (org_id+project_id)
fme_feature_flag_definition get / create / update (unchanged) get / create / update (unchanged) + list / delete / kill / restore / reallocate
fme_feature_flag create unchanged accepts traffic_type_id / traffic_type aliases; fails closed if name is missing

Native contracts (new in this PR)

  • listGET /fme/api/v4/feature-flag-definitions. Requires feature_flag_name. Pagination is limit / offset (default 100, max 100). No environment_id — definitions are listed across environments for that flag.
  • deleteDELETE /fme/api/v4/feature-flag-definitions/{feature_flag_name} with environment_id as a query param.
  • kill / restore / reallocatePOST /fme/api/v4/feature-flag-definitions/{feature_flag_name}/{kill|restore|reallocate} with environment_id as a query param. Optional body comment / title (same schema as flag-level kill/restore/reallocate). High-write, no retry.

workspace_id on list/delete/kill/restore/reallocate throws: Harness-native (org_id/project_id) only — MCP never supported workspace_id for this operation (get/create/update remain dual-mode). Mixed workspace_id + org_id/project_id is still rejected by the existing dual-mode helper.

Kill/restore/reallocate on this resource hit the same native endpoints as fme_feature_flag; either resource works.

Native flag create aliases (absorbed from #830)

Agents still pass legacy top-level traffic_type_id. Native v4 requires trafficType in the JSON body. This PR maps body.trafficType | traffic_type_id | traffic_type | body.traffic_type into trafficType on POST /fme/api/v4/feature-flags (never in the path), and throws if name is missing instead of POSTing undefined.

Not included from #830: the rollout-prompt NYI rewrite (already handled here / in #838) and the generic ResolvedRoute.headers fixture (merge already shipped in #806).

Key implementation changes

  • src/registry/toolsets/feature-flags.ts plus unit tests. No registry/index.ts or types.ts changes.
  • resolveNativeOnlyDefinitionRoute (immediately above featureFlagsToolset) for the five new ops. Get/create/update still use resolveFmeDualMode.
  • Resource and operation descriptions updated so agents see list/delete/execute as native-only vs dual-mode get/create/update, without HTTP paths in tool-facing text.
  • listFilterFields: required feature_flag_name, optional offset/limit.
  • Execute actions reuse fmeFeatureFlagKillRestoreReallocateSchema and fmeActionExtract.
  • Native fme_feature_flag.create bodyBuilder accepts traffic-type aliases and requires name.
  • src/prompts/feature-flag-rollout.ts: native-mode caveat no longer claims definition ops are unimplemented (rollout-status still is on this branch; feat: wire native FME traffic type and rollout status lists #838 wires that list).
  • README Feature Flags matrix and docs/testing/fme_feature_flag_definition/test_plan.md cover native-only list/delete/execute.

Type of Change

  • New feature
  • Bug fix (native create aliases)

Checklist

  • Unit: tests/registry/fme-feature-flag-definition-native-only.test.ts (identifier guards for list/delete/kill) + tests/registry/feature-flags.test.ts dual-mode get/create/update and create aliases
  • pnpm docs:check passes (README matrix)

Coding Standards (registry-driven MCP model)

  • No new server.registerTool() calls — only toolset definitions in src/registry/toolsets/
  • Toolset registered in ALL_TOOLSETS (unchanged — existing feature-flags toolset extended)
  • operationPolicy on every new/changed endpoint
  • Shared response extractors from src/registry/extractors.ts
  • identifierFields and scope declared
  • No console.log() in src/

Test plan

  • Native list uses feature_flag_name + limit/offset and does not put environment_id in the path
  • workspace_id rejected on list/delete/kill/restore/reallocate
  • Native delete + kill/restore/reallocate hit /fme/api/v4/feature-flag-definitions/{name} (with action suffix for execute)
  • Missing feature_flag_name on list and missing environment_id on delete/kill fail before any request
  • Native create with top-level traffic_type_id POSTs { name, trafficType } with no trafficTypes path segment
  • Native create without name throws before any request
  • Live smoke against qa.harness.io on a throwaway flag (create definition → list → execute → delete leftover)

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@ravindraharness
ravindraharness marked this pull request as ready for review August 18, 2026 11:10
@cursor

cursor Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Review — feat: wire native FME feature flag definition list, delete, and execute

Verdict: Approve with changes — implementation is solid and matches existing FME patterns, but README/docs updates are missing (and docs:check fails locally).

What's good

  • Scoped follow-up to feat: [FME-17249]: FME dual-mode Harness-native scope migration #806 — only adds the five native-only ops (list / delete / kill / restore / reallocate); get/create/update untouched.
  • resolveNativeOnlyDefinitionRoute cleanly rejects workspace_id with a clear error and reuses resolveFmeDualMode for mixed-scope rejection.
  • Routing looks correct: list → GET /fme/api/v4/feature-flag-definitions with feature_flag_name + pagination; delete/execute → per-flag paths with environment_id as query param (no /environments/ segment).
  • Consistent with siblings: reuses fmeFeatureFlagKillRestoreReallocateSchema, fmeActionExtract, and the same operationPolicy / body-builder pattern as fme_feature_flag execute actions.
  • Tests: new file covers list routing, delete, execute (all 3 actions), and workspace_id rejection. pnpm test (114 tests in the two files), pnpm standards:check, and pnpm build all pass on the PR branch.

Blocker — docs

PR checklist says docs:check passes, but on the branch it fails:

README.md Feature Flags table operation drift for fme_feature_flag_definition.list
README.md Feature Flags table operation drift for fme_feature_flag_definition.delete
README.md Feature Flags table execute actions for fme_feature_flag_definition are missing: kill, restore, reallocate

Update the Feature Flags matrix row for fme_feature_flag_definition and the prose bullet (~line 1509) to document native-only list/delete/execute.

Suggestions (non-blocking)

  1. Validation tests — add cases for missing feature_flag_name on list and missing environment_id on delete/execute (similar to existing fme_feature_flag guard tests).
  2. Stale agent-facing docssrc/prompts/feature-flag-rollout.ts still warns that native fme_feature_flag_definition "is not yet implemented"; docs/testing/fme_feature_flag_definition/test_plan.md still says list/delete/execute unsupported.
  3. Agent guidance — native kill/restore/reallocate now exist on both fme_feature_flag and fme_feature_flag_definition (same endpoints). Consider a one-liner in descriptions on when to prefer each.

CI

No status checks reported on the branch yet — worth confirming CI green after the README fix.

ravindraharness and others added 2 commits August 20, 2026 10:42
docs: check failed because the README still omitted native-only list/delete/execute; this also adds identifier-guard tests and drops the stale native-mode caveat.
Co-authored-by: Cursor <cursoragent@cursor.com>
Agents still pass legacy top-level traffic_type_id; native create only read body.trafficType and could POST a missing name.

Co-authored-by: Cursor <cursoragent@cursor.com>
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.

2 participants