feat(mac): add MacGiverClient and facsWrapper for FACS integration (Phase 1, steps 2–3)#1600
Draft
ravverma wants to merge 24 commits into
Draft
feat(mac): add MacGiverClient and facsWrapper for FACS integration (Phase 1, steps 2–3)#1600ravverma wants to merge 24 commits into
ravverma wants to merge 24 commits into
Conversation
|
This PR will trigger a minor release when merged. |
…hase 1, steps 2–3) - Add spacecat-shared-mac-giver-client package with MacGiverClient (getPermissions + checkPermission via POST /api/facs/permissions/check) and macGiverClientWrapper; defaults MACGIVER_BASE_URL to localhost:8080 - Add AuthInfo.getFacsPermissions() and hasFacsPermission() to http-utils - Add facsWrapper to http-utils: enforces FACS permissions per route with internal-identity bypass, internal-org bypass, LD feature-flag gate, deny-by-default for unmapped routes, and x-product header composition - Export FF_MAC_FACS_PERMISSIONS and X_PRODUCT_HEADER from constants Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… with current guard message Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ACS_EXCEPTION_INTERNAL_ORGS
- FF_MAC_FACS_PERMISSIONS is now a map from uppercased product code to flag key
(e.g. { LLMO: 'FT_LLMO-3026' }); products without an entry bypass FACS
- Wrapper resolves x-product header first, looks up the per-product flag, and
bypasses (without calling LD) when no flag is configured for the product
- Rename internal-org bypass env var INTERNAL_IMS_ORG_IDS to FACS_EXCEPTION_INTERNAL_ORGS
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ervice token Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…PERMISSIONS Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- src/auth/facs-wrapper.js: remove the two TEMPORARY logs added earlier for MAC rollout validation (the facsWrapper-entry debug log and the per-request permission-granted info log). Hot-path denials still emit warn() logs; bypasses keep their existing debug() logs. The success path is now silent. - test/auth/facs-wrapper.test.js: drop the orphaned grant-log assertion. 130 wrapper tests still pass; package suite 385/385 with 98.9% branch coverage (above 97% threshold). - src/auth/constants.js: fix the LLMO FACS flag key from FT_LLMO-3026 to FF_LLMO-3026 to match the actual LaunchDarkly flag. Co-Authored-By: Claude Sonnet 4.7 <noreply@anthropic.com>
…olver
Phase 2 of the MAC/FACS integration adds resource-scoped (ReBAC) checks
on top of the Phase 1 JWT permission claim. This commit ships the
shared-http-utils side: the resolver, the wrapper hook, and the small
route-utils helpers the resolver needs. Consumer (api-service) wires up
a stateLayerReader callback that hits PostgREST.
What changed
- New facs-resource-resolver.js exposes:
- buildAliasLookupsPerProduct(productsResourceParamAliases): inverts
the per-product resource -> [aliases] map into
Map<product, Map<alias, resourceType>>. Done once at wrapper
construction. Throws when an alias is declared under multiple
resources WITHIN the same product (config typo); same alias under
different products is allowed (the whole point of per-product scope).
- resolveFacsResource({ productCode, routePattern, params, body,
aliasLookupsPerProduct }): LIFO scan over URL params (rightmost
ReBAC-relevant wins); body fallback ONLY when the route has zero
ReBAC URL params for this product (prevents body-based spoofing -
same safety rationale as readOnlyAdminWrapper). Returns null when
the product has no ReBAC scope or no resolvable resource - signals
facsWrapper to skip the state-layer check.
- facsWrapper extended with an optional stateLayerReader callback. When
supplied, the wrapper resolves the request's resource and asks the
reader for a user-scoped grant first, then falls back to org-scoped.
Missing mapping -> 403. Reader errors are caught and surfaced as 403
(fail-closed). Routes without a resolvable resource (listings,
queries, the management endpoints themselves) skip this step entirely.
Phase 1 behaviour is unchanged when stateLayerReader is not supplied.
- route-utils.js gains two helpers:
- findMatchedRouteKey(context, routeMap): returns the matched route
pattern key (e.g. 'GET /sites/:siteId/llmo/config') for the current
request, so callers can do further structural work on the pattern
itself. Used by the wrapper to feed routePattern into the resolver.
- extractParamNamesInOrder(routePattern): parses ':param' names from
a route pattern in declaration order (left-to-right). The resolver
LIFO-scans the returned array.
- Wrapper docstring updated with the Phase 2 hook section and the new
bypass-rules numbering. JSDoc reflects the new optional opts.
Coverage / tests
- 422 package tests pass. 100% statements / 100% branches /
100% functions / 100% lines on facs-resource-resolver.js,
facs-wrapper.js, and route-utils.js.
- New test/auth/facs-resource-resolver.test.js covers buildAliasLookups
per-product (including the duplicate-alias throw and the empty / null
edge cases) and resolveFacsResource (LIFO scan, body fallback,
body-spoofing prevention, non-string coercion, all null returns).
- test/auth/facs-wrapper.test.js gets a new "Phase 2 state-layer check"
describe block: no-call when route has no resource, user-scoped hit,
org-scoped fallback, no-user-id -> org-only, deny-on-missing,
fail-closed on reader throw, body resolution, hook off when reader
absent, and construction-time alias collision throw.
- test/auth/route-utils.test.js adds describe blocks for
findMatchedRouteKey (exact / parameterised / no-match / no-method /
no-path) and extractParamNamesInOrder (ordered output, no-params,
non-string input, malformed input).
Co-Authored-By: Claude Sonnet 4.7 <noreply@anthropic.com>
Drop the `stateLayerReader` callback option from facsWrapper. The wrapper
now reads `context.dataAccess.services.postgrestClient` directly and calls
a new `findFacsAccessMapping` helper in `src/auth/facs-state-layer.js`.
Why: every service that mounts facsWrapper issues the exact same query
against the exact same `facs_access_mappings` table (schema lives in
mysticat-data-service and is fixed). A callback indirection just split
the lookup into two places (api-service support module + the callback
shim) without adding any flexibility. One source of truth here keeps the
wrapper-side authorisation logic in lock-step with the table shape.
- New src/auth/facs-state-layer.js: `findFacsAccessMapping(postgrestClient,
keys)`. Pure PostgREST point read, uses the composite index on
(ims_org_id, subject_type, subject_id, facs_permission, resource_type,
resource_id). Returns null when the row doesn't exist; throws with a
meaningful message when PostgREST returns an error.
- facsWrapper:
- drop the stateLayerReader option
- import findFacsAccessMapping and call it directly
- guard for missing postgrestClient on context (no dataAccess wrapper
upstream): log debug and skip the state-layer check rather than crash
- same fail-closed semantics on errors and missing-mapping
- New test/auth/facs-state-layer.test.js covers all five branches of
findFacsAccessMapping (data present, null, undefined, every key passed
as .eq filter, error propagation).
- facs-wrapper.test.js Phase 2 block rewritten to mock
findFacsAccessMapping via esmock and pass postgrestClient on context.
Added a new test for the "postgrestClient missing on context" skip path.
127 + 5 + ... = 427 + 6 = 433 passing across the package. 100%
statements / branches / functions / lines on facs-wrapper.js,
facs-resource-resolver.js, facs-state-layer.js, and route-utils.js.
Co-Authored-By: Claude Sonnet 4.7 <noreply@anthropic.com>
Three changes to facsWrapper to match the updated MAC state-layer design.
1. Route values are always string[] (no string-or-array)
Every value in PRODUCTS_ROUTES[<PRODUCT>] is now a non-empty array of
fully-qualified permissions. Single-permission routes use a one-element
array. The wrapper never has to branch on Array.isArray() per request.
Validation runs at wrapper construction - a misconfigured map (non-array,
empty array, non-string entry, permission without "<product>/<action>"
shape) throws at startup, not at the first request.
2. Held-permission resolution with exempt preference (Option B)
When a route lists multiple permissions, the wrapper picks one "held
permission" using two-pass logic:
- if the user holds any state-layer-exempt permission the route lists,
pick that one (regardless of array order). State-layer is skipped.
- otherwise pick the first held permission. State-layer enforcement
runs with that permission as the facs_permission lookup key.
Without exempt preference, an llmo_manager holding both can_view (brand-
scoped) and can_view_all (exempt) on a route listed as
['can_view', 'can_view_all'] would be forced through the state-layer
check on can_view, denying the universal access can_view_all grants.
Exempt preference makes the wrapper robust to listing order.
3. PRODUCTS_FACS_STATE_LAYER_EXEMPT_PERMISSIONS config field
New per-product field on routeFacsCapabilities. The wrapper builds a
Map<product, Set<permission>> at construction. Per-product so each
product decides which of its permissions are global by design.
Tests (149 passing across the wrapper file, full package 440 passing with
100% coverage on facs-wrapper.js, facs-resource-resolver.js,
facs-state-layer.js, and route-utils.js):
- updated existing fixture to array form
- new "construction-time validation" describe block with 5 cases (string,
empty array, missing prefix, non-string entry, undefined product/exempt)
- new "any-of permissions" block: first-match-wins fallback, second-perm
fallback, deny on none-held
- new "state-layer exempt permissions" block: skip on can_view_all,
enforce on can_view, skip on can_manage_user, exempt-preferred when user
holds both, brand-scoped when user holds only the non-exempt permission
Co-Authored-By: Claude Sonnet 4.7 <noreply@anthropic.com>
… evaluate" getPermissions previously collapsed every non-2xx response from MacGiver into the same empty-array result as a legitimate "user has no FACS roles". That made misconfigured integrations, transient 5xx, and 4xx auth failures indistinguishable from a real "no permissions" outcome — ops alerting and incident forensics had no signal that MacGiver was degraded. This change: - getPermissions now logs the non-2xx with `tag: 'macgiver'` + status + userId + imsOrgId, and throws. Callers that want to distinguish the two outcomes can handle the throw; login.js's existing try/catch fires and omits the facs_permissions claim from the JWT, which is the right fail-safe behaviour at login (user signs in without FACS perms, the wrapper denies FACS-governed routes for them). - checkPermission stays as a boolean convenience wrapper. It now explicitly catches the throw and returns false — preserving the fail-closed boolean contract for callers that don't care about the distinction. getPermissions already logged the underlying cause, so the catch is silent. Tests: - The non-2xx case in getPermissions is updated from "returns []" to "throws and logs" with assertions on the warn fields. - The checkPermission non-2xx test stays unchanged (still returns false) but now exercises the explicit catch path. - chai-as-promised + sinon-chai wired locally to express the throw and log-spy assertions; both were already in devDependencies. Coverage held at 100% lines/statements/branches/functions.
…inding-only shape
Three coordinated renames that align facsWrapper with the new state-layer
design (mac-state-layer.md §"State Layer Schema" / §"State Layer Data Access"):
JWT carries capability (single source of truth), state layer stores only
subject↔resource bindings, request-time lookup answers existence only.
constants.js
- FF_MAC_FACS_PERMISSIONS → FT_MAC_FACS_PERMISSIONS. The constant name
uses the FT_ prefix to match the LaunchDarkly key naming convention.
Values are unchanged (LD keys stay live).
- Added a comment block explaining the "flag absent → enforcement
universal (retired)" semantic so future contributors don't add an entry
back as a "default".
facs-state-layer.js
- findFacsAccessMapping → findFacsResourceBinding.
- Dropped the facsPermission arg from the signature. The lookup no
longer filters on facs_permission; the state layer doesn't store
capability under the new schema.
- Selects only `id` (existence check, no payload needed).
- Added `.is('revoked_at', null)` filter so tombstones never match.
Backed by the partial active-row index on the table.
facs-resource-resolver.js
- Body-fallback precondition tightened: body is consulted only when
the route declares ZERO URL path params. Previously consulted when
no ReBAC-relevant URL params existed, which let a non-ReBAC param
(e.g. spaceCatId) leave the body open as a sibling resource id —
widening the spoofing surface relative to readOnlyAdminWrapper's
precondition. The two resolvers now use the same rule.
facs-wrapper.js
- Updated to import the renamed symbols and call findFacsResourceBinding
without facsPermission. heldPermission stays in forensic logs only.
Tests
- findFacsResourceBinding test: asserts the .is('revoked_at', null)
filter is set, asserts only `id` is selected, asserts no
facs_permission column appears in the .eq() chain.
- Resolver body-fallback tests: existing "any ReBAC URL param blocks
body" test kept; added a new test that a non-ReBAC URL param ALSO
blocks body (regression guard for the precondition tightening); the
"zero URL params" case is now the only path that consults body.
- Wrapper tests: stub renamed, assertions checking keys.facsPermission
changed to assert the key is absent.
All 443 tests passing. Coverage 100% lines/statements/functions, 99%
branches per package threshold.
Out of scope for this commit (separate commit follows):
- Bypass-ladder fail-closed changes (no x-product → 403; LD failures
→ 503), tenant assertion, resolveUserIdent narrowing — all in
facs-wrapper.js, but a distinct concern with bigger test surface.
…sertion + sub-only ident Implements the design's fail-closed bypass ladder (mac-state-layer.md §1.5 "Bypass order (fail-closed)" and "Enforcement floor") in facs-wrapper.js. Three behaviour changes plus a small assertion: 1. No x-product on a FACS-governed route → 403 (was: bypass). The wrapper now distinguishes "route in some product map" from "route in no product map at all". A route that no product claims continues to bypass (it's not FACS-governed; e.g. /heartbeat, INTERNAL_ROUTES entries by the coverage invariant). A route that IS in some product map but where x-product is missing or names a product that doesn't list it returns 403 — the customer-controlled header cannot be used to silently disable FACS by omission. 2. LaunchDarklyClient.createFrom failure → 503 (was: bypass). Same for isFlagEnabledForIMSOrg rejection. The wrapper cannot determine flag state, so it cannot make a defensible allow/deny decision; the previous fail-open path would have silently downgraded enforcement on LD outages. 3. Flag entry absent at build-time → bypass the rollout gate AND proceed to enforcement (was: bypass the whole wrapper). Removing the per-product entry in FT_MAC_FACS_PERMISSIONS is the documented "flag retirement" mechanism: enforcement becomes universal for that product. The previous behaviour treated a missing entry as "no enforcement", which would have disabled FACS silently if anyone removed the entry to clean up. 4. authInfo.getTenantIds() returning more than one tenant → 500. Per the design's Tenant scoping subsection, the SpaceCat login flow always populates exactly one tenant; a length > 1 is a configuration violation that should surface in operational alerting rather than have the wrapper silently pick tenants[0]. 5. resolveUserIdent returns profile.sub only (was: sub || email). Auth-service canonicalizes profile.userId === sub === email at login. The wrapper picks sub deterministically; if sub is missing (legacy JWT issued before canonicalization, or a different auth path), the wrapper logs the user as undefined rather than reaching for email. Helpers - serviceUnavailable(message) and internalServerError(message) inline next to the existing forbidden(message) helper. - routeMatchesAnyProductMap(context, productsRoutes) determines whether a request would hit any product's sub-map; used to distinguish "FACS-governed without x-product" (403) from "not FACS-governed" (bypass). Dead-code cleanup - The "deny by default: unmapped routes within an enrolled product" branch is removed. Under the new ladder, by the time the wrapper reaches the held-permission step, routeInThisProduct has already established the route is in the current product's map. Tests - 10 prior tests rewritten to match the new contract (fail-closed expectations replacing fail-open). - 4 new tests added: tenant > 1 → 500; LD ctor returns null → 503; flag retirement (entry absent) proceeds to enforcement instead of bypassing the wrapper; no x-product on non-FACS route still bypasses. - INTERNAL_ROUTES tests updated to reflect the new "controllers gate, wrapper bypasses" model (matches §"Controllers are not modified by this design"). - resolveUserIdent tests reflect the sub-only contract; the fallback test became a "missing sub → undefined" test. 448 tests passing; coverage 100% lines/statements/functions, 99% branches per the package threshold; facs-wrapper.js itself at 100%/100%/100%/100%.
Re-exports the FACS rollout constants from the package entry so consumers (notably spacecat-auth-service login.js) can use the same flag table as facsWrapper instead of re-declaring it locally. No behaviour change in facsWrapper; only the public API surface widens. Coverage held; 448 tests passing.
…uth types
The legacy api-key handler sets withType('legacyApiKey') and a profile
of { user_id: 'admin' } or { user_id: 'legacy-user' } — NO `is_admin: true`
flag. After the fail-closed bypass change in 6347052, api-key requests
that hit a FACS-mapped route without an x-product header (which the
admin/test/internal traffic does) get 403, even though api-key auth is
an internal trust path and not the target of FACS at all.
Add the api-key types to the identity-bypass step (1) alongside
is_admin / is_s2s_admin / is_s2s_consumer / is_read_only_admin.
Matches how S2S consumer JWT bypasses: same intent (internal trust),
different transport.
Symptom this fixes: 11 `Index Tests` in spacecat-api-service now
return 403 instead of the expected 400/200 because the wrapper's
fail-closed gate (no x-product on a FACS-mapped route) fires for
api-key requests that pre-FACS would have passed through.
Tests: two new bypass tests (legacyApiKey + scopedApiKey); 450/450
passing; 100% coverage on facs-wrapper.js.
Add a bypass/grant info log at each decision point in facsWrapper so operators can trace why FACS allowed (or fell through to) a request without re-reading the wrapper source. Every log includes the request method and suffix so a single grep yields the full FACS verdict for a given route. Bypass logs: options-preflight, internal-identity (with auth flags), internal-adobe-org, route-not-facs-governed, ld-flag-off, and the flag-retired fallthrough. Grant logs: state-layer-exempt, no-postgrest-client, state-layer-binding (with resource + via), no-resolvable-resource. The four existing tests that asserted on the debug-level placeholders are updated to expect the new info logs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
helix's enrichPathInfo only populates pathInfo.method / headers / route / suffix — it does NOT extract :param values. Path-param extraction happens in the route matcher AFTER the middleware chain completes, so reading context.pathInfo.params from inside facsWrapper always yielded undefined. The LIFO scan in resolveFacsResource then found the alias (e.g. brandId to brand) but read undefined for its value, fell through to the body fallback which is gated by routeParams.length === 0, and returned null. Result: every URL-bound resource request logged "no-resolvable-resource" and skipped Phase 2 entirely, granting on the JWT claim alone — even when no state-layer binding existed for the user / org against that resource. readOnlyAdminWrapper already worked around this gap by calling extractRouteParams itself; facsWrapper now does the same. The helper lives in route-utils.js and is the same one readOnlyAdminWrapper uses, so behaviour stays consistent across the two wrappers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… state-layer lookups The state-layer schema stores `facs_access_mappings.ims_org_id` in canonical `<ident>@<authSrc>` form (per `platform/decisions/mac-state-layer.md` §"Org identifier"), but `authInfo.getTenantIds()[0]` returns the BARE ident. Without normalization the wrapper filtered `ims_org_id = <bare>` against rows stored `<bare>@AdobeOrg` and never matched — every brand-scoped Phase 2 request fail-closed with 403 even when a binding existed. Add `normalizeImsOrgId(orgIdent, authSrc = 'AdobeOrg')` to `auth/facs-state-layer.js` and export from the package index. Idempotent: returns the input unchanged when it already carries an `@<authSrc>` suffix, or when it's null / undefined / non-string. Default suffix `'AdobeOrg'` matches the precedent already in `readOnlyAdminWrapper` and the `LaunchDarklyClient.isFlagEnabledForIMSOrg` call site here. `facsWrapper` now normalises `orgId` once via `canonicalImsOrgId = normalizeImsOrgId(orgId)` before the user-scoped and org-scoped `findFacsResourceBinding` calls. The grant log reports the canonical org id (forensic value). Unit tests cover: bare ident -> @AdobeOrg, idempotent on already-canonical input, explicit authSrc override, no double-suffixing, null/undefined/empty/ non-string pass-through. 458 tests pass at 100% coverage on facs-wrapper.js and facs-state-layer.js. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ravverma
force-pushed
the
feat/mac-facs-integration
branch
from
5f952c7 to
8189016
Compare
The wrapper now implements the additive grant-only evaluation rule:
effectiveCapabilities(user, resource, product) =
JWT.facs_permissions(user, product)
∪ stateLayer.granted_capabilities(user, resource, product)
∪ stateLayer.granted_capabilities(org, resource, product)
admit iff routeCapability ∈ effectiveCapabilities
Removed from the wrapper contract:
- PRODUCTS_FACS_ADMIN_PERMISSIONS (product-admin bypass) — gone
- PRODUCTS_FACS_STATE_LAYER_EXEMPT_PERMISSIONS (exempt list) — gone
- Per-route any-of array (each route guards one capability) — gone
Universal grants now flow through state-layer org-scoped bindings whose
granted_capabilities carry the same string as the JWT claim would have.
The wrapper unions both sources; no special-case bypasses needed.
Resource resolution: when resolveFacsResource returns null (no ReBAC
anchor on the route OR the request's resource is not modelled as a
ReBAC entity for the product), the wrapper defers to the controller
rather than denying. Routes that legitimately operate on non-ReBAC
entities (e.g. LLMO suggestion writes) continue to work; controller-
side authorisation remains the gate.
State-layer lookup updates:
- findFacsResourceBinding now filters by `product` (new schema column)
- Returns `id, granted_capabilities` rather than just `id`
- resolveFacsResource gains a `query` fallback so the
/state/access-mappings/* URL grammar (resource carried in query on
listings) is recognised
Tests fully rewritten: 47 wrapper / 16 state-layer / 19 resolver tests
covering the new bypass ladder, hybrid additive evaluation, no-postgrest
JWT-only fallback, and the query-fallback path. 100% statements / 100%
functions / 100% branches across the three rewritten files.
See platform/decisions/rebac-hybrid-permission-model.md and
mac-state-layer.md "State Layer Evaluation Engine".
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…AUTH CI's preflight guard requires every .releaserc.cjs to declare @semantic-release/npm conditionally so the workflow can disable npm auth during dry-run / no-publish releases. Mirrors the pattern in every other package config in the monorepo (e.g. spacecat-shared-http-utils, spacecat-shared-data-access). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The lock file was missing entries for the new workspace package @adobe/spacecat-shared-mac-giver-client@1.0.0 and a handful of its dev deps (nock, sinon, @sinonjs/samsam, diff, type-detect). CI's npm ci step fails with EUSAGE when the lock file is out of sync with the package.json workspaces graph. Regenerated by running npm install at the repo root. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The wrapper computed the full effective set (JWT union state-layer) on
every request: it always resolved the ReBAC resource and always issued
the two findFacsResourceBinding reads, then unioned with the JWT at the
end. When the JWT already carried the route capability, that work could
not change the outcome — the union already contained the capability — so
the resource resolution and both DB round-trips were pure overhead.
Reorder to evaluate the union lazily, matching the design intent:
5. JWT short-circuit. If the JWT carries the route capability, admit
immediately (grant: 'jwt') and skip the resolver + state layer.
6. Otherwise resolve the resource; null -> defer to controller.
7. Read the state layer; granted iff a row carries the capability
(the JWT was already ruled out). No postgrestClient + no JWT grant
-> 403 (the only remaining admit path is unreachable).
This also removes the confusing second JWT re-check that previously lived
on the no-postgrest branch (old 'jwt-only-no-postgrest' grant tag): the
JWT is now checked exactly once, up front. Behaviour is unchanged — the
effective set is still JWT union state-layer — only the evaluation order
and the avoided computation differ.
facs-wrapper.js: 100% statements / branches / functions / lines.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Please ensure your pull request adheres to the following guidelines:
Related Issues
Thanks for contributing!