fix: [FME-17257]: point fme.spec.yaml at real v4 FME endpoints - #111
Draft
puthrayaharness wants to merge 5 commits into
Draft
fix: [FME-17257]: point fme.spec.yaml at real v4 FME endpoints#111puthrayaharness wants to merge 5 commits into
puthrayaharness wants to merge 5 commits into
Conversation
pkg/spec/fme.spec.yaml declared feature_flag/feature_flag:definition commands against /v3/feature-flag... paths with entity-wrapped JSON, but the live FME API is /fme/internal/api/v4/... and returns flat items with no entity wrapper. Swap the path prefix, drop every it.entity indirection, and fix the killed->isKilled and modifiedAt->createdAt field drift so list and get actually work against the real API. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> AI-Session-Id: 20b23a2d-5d3f-4d25-9879-167a341599d4 AI-Tool: claude-code AI-Model: unknown
|
Deepak Puthraya seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
added 3 commits
August 12, 2026 15:15
…pe flag
Live verification against qa.harness.io surfaced two more real-API drifts
beyond the read-path fixes in the previous commit:
- POST/PATCH responses for feature_flag and feature_flag:definition wrap
the resource in {"entity": {...}, "governance": {...}}, unlike the flat
GET/list shape. item_expr now reads it.entity on every create/update/
archive/unarchive/kill/restore/reallocate command.
- `flags.traffic-type` parsed as `flags.traffic - type` under expr-lang
(hyphens aren't valid in identifiers), so create_body_init silently
dropped trafficType from the POST body. Switched to bracket indexing
(flags["traffic-type"]), matching the convention already used for
hyphenated flags elsewhere in the codebase.
Also added mutable_path to feature_flag's description field — update
feature_flag had zero mutable fields declared, so `--set` always failed
with "unknown or read-only field" regardless of path.
Verified live: full create/update/archive/unarchive/delete lifecycle for
feature_flag, and create/list/get/update/kill/restore/reallocate/delete
for feature_flag:definition, against qa.harness.io project puthraya with
a disposable flag (cleaned up after). go test ./... and modules/har's
suite both pass.
Known follow-up not fixed here: `update feature_flag:definition --set
trafficAllocation=...` (the example in the command's own help text)
still fails — MutableFields only reads noun.Fields, not the definition's
fields_extra, so there's no way to expose a definition-specific mutable
field without a registry/spec engine change. Out of scope for this
mechanical spec fix.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
AI-Session-Id: 20b23a2d-5d3f-4d25-9879-167a341599d4
AI-Tool: claude-code
AI-Model: unknown
…n FME v4 commands Extends the v4-shaped fme.spec.yaml with segment (CRUD), fme_environment (CRUD; named to avoid colliding with the CD module's "environment" noun), segment:definition (CRUD, scoped by --env), and segment:definition_keys (list/add/remove) commands, following the same real-API-verified approach used for feature_flag. Live-verified against qa.harness.io: fme_environment and segment full CRUD, and segment:definition list. segment:definition writes and the keys add/remove commands could not be live-verified in this session due to an in-progress backend rollout (PR #12644) causing intermittent 403/405 responses; the request shapes are believed correct based on the 403 responses matching the documented error contract. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> AI-Session-Id: 20b23a2d-5d3f-4d25-9879-167a341599d4 AI-Tool: claude-code AI-Model: unknown
…dation
fields_noun belongs on CommandSpec, not nested under endpoint: — the
embedded-spec loader rejected the file with strict YAML unmarshaling
("field fields_noun not found in type spec.EndpointSpec"), failing
"Build, Test & Validate Specs" in CI. Also suppresses the expected
"missing get_id_expr" warning on segment:definition_keys (raw string
items have no id), following the same get_id_expr: "-" convention used
elsewhere in the repo.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
AI-Session-Id: 20b23a2d-5d3f-4d25-9879-167a341599d4
AI-Tool: claude-code
AI-Model: unknown
…ay-flag bug, blank definition fields Backend dropped /internal/ from all FME v4 endpoint paths (/fme/internal/api/v4/... -> /fme/api/v4/...); update all commands and regression tests accordingly. Live end-to-end testing against qa.harness.io surfaced two further bugs: - buildctx.go's required-flag check called cmdctx.GetString on every flag regardless of type, but is_array/is_multi flags are stored as []string in FlagValues, so GetString always returned "" and any required+is_array (or is_multi) flag was permanently rejected as missing. Only segment:definition_keys_remove's --keys combined required+is_array, so this was a latent, previously-unexercised engine bug. Fixed the check to branch on flag type, with a regression test. - get/create/update/kill/restore/reallocate feature_flag:definition and get/create/update segment:definition rendered blank fields (Name, Description, Traffic Type, ...) because they fell back to the base feature_flag/segment noun fields, which don't apply to a definition's shape. Added dedicated feature_flag_definition and segment_definition nouns (mirroring the existing segment_definition_keys pattern) and wired them via fields_noun. Also added fields_noun to segment:definition_keys_add/_remove, which had the same issue, and fixed update feature_flag:definition's --set traffic_allocation (was undocumented/unmapped, causing "unknown or read-only field"). update segment:definition still returns an intermittent 500 from the backend on a minimal, correct merge-patch body — confirmed backend-side, no CLI/spec change addresses it. AI-Session-Id: 20b23a2d-5d3f-4d25-9879-167a341599d4 AI-Tool: claude-code AI-Model: unknown
2 tasks
ravindraharness
added a commit
to ravindraharness/cli
that referenced
this pull request
Aug 13, 2026
Rebased onto Deepak's latest harness#111 (public /fme/api/v4 paths, fields_noun definitions, required-array flags). Remaining CLI-only: RFC 7807 detail in API errors, MutableFields includes fields_extra, env-delete help for hasDependents/API tokens. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
@cursoragent review this PR |
Collaborator
|
@puthrayaharness looks good! let me know when you want this merged |
Contributor
Author
|
Our BE changes need to be rolled out. Few of the changes are still pending. Will change from Draft to Ready to review when we are good. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
fme.spec.yamlpointed every FME command at the wrong API (/v3/...,entity-wrapped responses). Fixedfeature_flagagainst the real API, then extended the spec withsegment,fme_environment,segment:definition,segment:definition_keysusing the same pattern.Backend later dropped
/internal/from every FME v4 path (/fme/internal/api/v4/...→/fme/api/v4/...); this PR tracks that too.feature_flagfixes/v3/feature-flag.../fme/api/v4/...entity-wrapped.entity(it.entity.X→it.X)entity-wrapped ({"entity":..., "governance":...})item_expr: it.entityon mutation commandsflags["traffic-type"]parsed asflags.traffic - typeunder expr-langkilled→isKilled, nomodifiedAton definitionsit.isKilled;createdreadsit.createdAtupdate feature_flag --set description=...had no mutable fieldmutable_path: descriptionNew resources
segmentname, likefeature_flagfme_environmentfme_environment, notenvironment— that noun/alias belongs to the CD module. Addressed by UUID (it.id), not namesegment:definition--env;listhas no parent-id requirement (env-scoped, not segment-scoped)segment:definition_keysexecute ...:definition_keys_add/_remove{"keys": [...], "governance": ...}(noentity) →item_expr: it.keysSoft-archive delete confirmed live:
DELETEreturns200with no body; subsequentGETon an archivedsegment/segment:definitionreturns404.Engine fixes (surfaced by live testing, not spec-specific)
execute segment:definition_keys_remove --keys k1always erroredflag --keys is required, even when passedbuildctx.go's required-flag check calledcmdctx.GetStringon every flag regardless of type;is_array/is_multiflags are stored as[]string, so it always read back""f.IsArray || f.IsMulti(GetStringSlice) vs. scalar (GetString); regression test addedget/create/update/kill/restore/reallocateonfeature_flag:definition, andget/create/updateonsegment:definition, rendered blankName/Description/Traffic Type/...feature_flag/segmentnoun fields, which don't describe a definition's shape (onlylisthad afields_extraoverride)feature_flag_definition/segment_definitionnouns (mirroring the existingsegment_definition_keyspattern) and wired all affected commands viafields_nounexecute segment:definition_keys_add/_removerendered blank fieldsfields_noun: segment_definition_keysupdate feature_flag:definition --set trafficAllocation=...→unknown or read-only fieldmutable_path, and the documented flag name didn't match the field idmutable_path: trafficAllocationto thetraffic_allocationfield; corrected the--setexample totraffic_allocationTest plan
go test ./...,modules/har,check:specs:mainpkg/specloader/fme_spec_test.go,pkg/registry/buildctx_workflow_test.gofields_nounoverrides, required+array flag)feature_flagfull lifecycle (create/get/update/archive/unarchive/delete)qa.harness.iofeature_flag:definitionfull lifecycle (create/get/list/update/kill/restore/reallocate/delete)fme_environmentlist/get/updatedeleteblocked by a400 hasDependentsbusiness rule on the account's existing test data — expected, not a spec issuesegmentfull lifecycle,segment:definitioncreate/get/list/delete,segment:definition_keyslist/add/removeupdate segment:definition500from the backend on a minimal, correct merge-patch body ({"description": "..."}) — confirmed backend-side, not addressed by this PRNote: the original
403 forbiddenreported onsegment:definitionwrites was not an RBAC gap — it was caused by hitting the (now-removed)/internal/path variant, which was secured differently. Once switched to/fme/api/v4/..., the same calls succeed with the same PAT and no permission changes.