test: cover FME dual-mode gaps from PR #806 - #807
Closed
cursor[bot] wants to merge 12 commits into
Closed
Conversation
Introduces optional routeResolver field to enable per-call route resolution for supporting dual-mode scope contracts (e.g., FME legacy Split.io vs Harness-native). Adds ResolvedRoute interface and wires resolver into path-building, product resolution, and tracing logic. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> AI-Session-Id: 2ffa90c0-3343-42a0-a21a-504ff2c863d1 AI-Tool: claude-code AI-Model: unknown
AI-Session-Id: 2ffa90c0-3343-42a0-a21a-504ff2c863d1 AI-Tool: claude-code AI-Model: unknown
…ertions AI-Session-Id: 2ffa90c0-3343-42a0-a21a-504ff2c863d1 AI-Tool: claude-code AI-Model: unknown
Guard the scopeOptional org/project injection in registry/index.ts with a workspace_id check — FME's legacy Split.io identifier and Harness-native org/project are mutually exclusive scoping modes, and UI URLs incidentally carry org/project segments that must not leak into workspace_id-scoped calls. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> AI-Session-Id: 2ffa90c0-3343-42a0-a21a-504ff2c863d1 AI-Tool: claude-code AI-Model: unknown
Wires list/get/delete through a routeResolver + resolveFmeDualMode to route legacy Split.io workspace_id calls vs. Harness-native org_id/ project_id calls. create/update/kill/restore/archive/unarchive keep their legacy paths and throw a not-yet-implemented error in Harness-native mode. Also gates applyUrlDefaults's URL-derived org_id/project_id merge on explicit workspace_id for fme_* resources, since the two are mutually exclusive FME scoping modes and a UI URL's incidental org/project segments must not turn a legacy call into a mixed-mode error. Fixes the six harness_* tool handlers to merge params/filters into args before calling applyUrlDefaults so this gate can see explicitly passed identifiers, not just top-level named args. AI-Session-Id: 2ffa90c0-3343-42a0-a21a-504ff2c863d1 AI-Tool: claude-code AI-Model: unknown
…sed_segment dual-mode routing
Implement routeResolver for three FME resources following Task 4's pattern:
- fme_environment.list: legacy → /internal/api/v2/environments/ws/{wsId}; new mode → /fme/internal/api/v4/environments
- fme_standard_segment.list/get: legacy → /internal/api/v2/segments/ws/{wsId}[/{segmentName}]; new mode → /fme/internal/api/v4/segments[/{segmentName}]
- fme_rule_based_segment.list/get/delete: legacy → /internal/api/v2/rule-based-segments/ws/{wsId}[/{segmentName}]; new mode → /fme/internal/api/v4/segments[/{segmentName}]
- fme_rule_based_segment.create: not-yet-implemented error for Harness-native mode; passes workspace_id mode through
Remove required: true from workspace_id listFilterFields (now optional with deprecation notice matching fme_feature_flag pattern).
All 39 feature-flags tests pass; full suite: 2845 tests pass.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
AI-Session-Id: 2ffa90c0-3343-42a0-a21a-504ff2c863d1
AI-Tool: claude-code
AI-Model: unknown
…tors AI-Session-Id: 2ffa90c0-3343-42a0-a21a-504ff2c863d1 AI-Tool: claude-code AI-Model: unknown
AI-Session-Id: 2ffa90c0-3343-42a0-a21a-504ff2c863d1 AI-Tool: claude-code AI-Model: unknown
AI-Session-Id: 2ffa90c0-3343-42a0-a21a-504ff2c863d1 AI-Tool: claude-code AI-Model: unknown
…ope guards to FME, reject partial org/project pairs - Add requireFmeIdentifier() and use it at all 37 FME routeResolver path-segment sites so a missing identifier throws instead of building a malformed URL (routeResolver bypasses the registry's pathParams presence check). - Key the scopeOptional org/project suppression on product: "fme" instead of the bare presence of a field named workspace_id, so future non-FME scopeOptional resources keep their scope query params. - applyUrlDefaults now prefers the caller's declared resource_type over the URL-parsed one when deciding to skip URL org/project for legacy FME calls. - Add isFmeHarnessNativeSelected() for the 7 permissive mode-selector resolvers: a partial org_id/project_id pair is now rejected instead of leaking a stray orgIdentifier/projectIdentifier query param onto a legacy Split.io call. - Drop the nonexistent fme_standard_segment.create entry from the spec doc. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> AI-Session-Id: 2ffa90c0-3343-42a0-a21a-504ff2c863d1 AI-Tool: claude-code AI-Model: unknown
…ing, test_report overclaim Prompt: feature-flag-rollout now warns that fme_feature_flag_definition, fme_rollout_status, and kill/restore have no Harness-native route yet, so steps 3/4/7 only work with workspace_id today. README: fme_standard_segment has no create operation in either mode — previous wording implied one exists but isn't implemented yet. test_report: TC-023's "confirmed 200" claim narrowed to what was actually verified (mocked-client routing/header shape), not a live probe of the exact product:"harness" request shape. AI-Session-Id: 2ffa90c0-3343-42a0-a21a-504ff2c863d1 AI-Tool: claude-code AI-Model: unknown
Add regression tests for: - feature-flag-rollout prompt scope validation and NYI caveat - fme_feature_flag delete dual-mode routing and single deprecation log - harness_list filters.workspace_id URL scope precedence for legacy FME Co-authored-by: Rohan Gupta <thisrohangupta@users.noreply.github.com>
|
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. |
Collaborator
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
3 participants
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
Adds targeted regression tests for risky behavior introduced in #806 (FME dual-mode Harness-native scope migration) that was not yet covered.
Risky behavior now covered
feature-flag-rolloutprompt — scope validation (workspaceId vs orgId+projectId), Harness-native NYI caveat for steps that still require legacy modefme_feature_flagdelete routing — legacy Split.io path vs Harness-native/fme/internal/api/v4/feature-flags/{name}pathworkspace_idcalls log exactly once per dispatch (guards doubleresolveFmeDualModeinvocation)harness_listparams merge —filters.workspace_idis visible toapplyUrlDefaultsbefore URL org/project merge, preventing scope leakage on legacy FME list callsTest files added/updated
tests/prompts/feature-flag-rollout.test.ts(new)tests/registry/feature-flags.test.tstests/tools/tool-handlers.test.tsWhy these tests materially reduce regression risk
These paths sit at the boundary between two mutually exclusive scoping contracts (
workspace_idvsorg_id+project_id). A regression would either route to the wrong API, leak Harness scope params onto Split.io calls, or mislead agents via the rollout prompt. The tests are deterministic unit/integration tests with mocked HTTP — no flakiness observed.Validation