[CLOV-1618] Add reusable workflow for Backpack adoption guard#4680
Open
Richard-Shen (RichardSyq) wants to merge 25 commits into
Open
[CLOV-1618] Add reusable workflow for Backpack adoption guard#4680Richard-Shen (RichardSyq) wants to merge 25 commits into
Richard-Shen (RichardSyq) wants to merge 25 commits into
Conversation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zizmor's artipacked rule flagged the actions/checkout step in the UpdateMajorTag job for persisting the bot token via .git/config (the checkout default). Switch to persist-credentials: false and inject the token only at the moment of git push via http.extraHeader. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The CI Build job restores node_modules/ from a cache that only saves the root node_modules/ directory. When @actions/core is declared only under libs/backpack-adoption-action's package.json, npm installs it at libs/backpack-adoption-action/node_modules/@actions/core (nested), and that nested directory is dropped between Create-NPM-Cache and Build, breaking tsc with TS2307. Other action deps (@babel/parser, glob, typescript) hoist to root because root devDependencies already pin related packages. Adding @actions/core as a root devDependency at the same pinned version forces npm to hoist it. No source changes required. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
setup-node's node-version-file and cache-dependency-path inputs accept
only paths relative to GITHUB_WORKSPACE (the consumer repo). Passing
the absolute ${{ github.action_path }} path makes setup-node prepend
the workspace root to it, looking up a non-existent doubled path:
/home/runner/_work/<consumer>/<consumer>
/home/runner/_work/_actions/Skyscanner/backpack/.../
Hardcode node-version: "22" to match the monorepo's .nvmrc spirit and
drop cache-dependency-path. The npm cache fallback still works against
the consumer's lockfile when present.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When run inside a consumer repo whose CI sets NPM_CONFIG_USERCONFIG to a custom .npmrc (e.g. carhire-homepage's .npmrc.ci pointing at Skyscanner Artifactory with an auth token), our composite npm ci inherits that config and fails with E401 because NODE_AUTH_TOKEN is not in scope. All of this action's runtime dependencies are public on npmjs.org, so override the registry explicitly and bypass the consumer's userconfig by pointing it at /dev/null. This keeps our install fully isolated from the consumer's npm setup. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The composite-action approach kept hitting consumer-environment edge cases (workspace dep nesting, setup-node absolute-path handling, consumer .npmrc.ci forcing private registry without an auth token). Following Skyscanner/cypress-a11y-action, switch the action back to a single bundled JS action so it is fully isolated from the consumer's runtime npm/node setup. - action.yml: runs.using: node24, main: dist/index.js - package.json: build runs esbuild bundle; verify-dist re-builds and diffs the committed dist - dist/: bundled (~2.5 MB) and committed; sourcemap alongside - Drop tsconfig.build.json and the per-action package-lock.json (composite-only artefacts) - Restore project.json verify-dist target and release-workflow step - Drop /libs/backpack-adoption-action/dist gitignore entry Source remains babel-based with @actions/core, simplified className detection, and JSON.parse(readFileSync) for the GitHub event payload. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Danger flagged 18 new .ts and .js files under libs/backpack-adoption-action/ for missing the Backpack license heading required by dangerfile.ts. Added the standard Apache 2.0 header to each. Rebuilt dist/. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Revert the release-related workflow edits so this PR only adds the action source. The release strategy (action release workflow, tag-namespaced filtering on the backpack-web release workflow, verify-dist step) will be handled separately. - Restore .github/workflows/release.yml to origin/main (drop the 5 startsWith(...) filters that excluded backpack-adoption-action/* tags). - Restore .github/workflows/_build.yml to origin/main (drop the no-op verify-dist step; no project defines that target). - Delete .github/workflows/backpack-adoption-action-release.yml. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Make the action's identifier match its action.yml display name
("Backpack Adoption Guard") and move the project from libs/ alongside
the existing publishable packages.
- Move libs/backpack-adoption-action/ → packages/backpack-adoption-guard/.
- Rename npm package @skyscanner/backpack-adoption-action →
@skyscanner/backpack-adoption-guard.
- Rename Nx project backpack-adoption-action → backpack-adoption-guard.
- Update README usage path and nx run command.
- Update root .eslintignore and .prettierignore to reference the new path.
- Regenerate package-lock.json.
Verified with lint, typecheck, test (12/12), build, and verify-dist —
the bundled dist/ output is byte-identical after rebuild (package name
change does not affect the esbuild bundle content).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Now that the project lives at packages/backpack-adoption-guard, the root jest testRegex (?:packages|token-sync)/.*-test would otherwise try to run the action's tests in the root jsdom environment with React-oriented setup. The action has its own jest config (node environment, no React setup) and runs via nx's project test target, so root jest should skip it entirely. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Cortex upload responsibility is moved out of this action so consumers
can use the dedicated Skyscanner/push-custom-cortex-data action instead
of duplicating webhook plumbing here. The guard action now only:
- Calculates Backpack adoption.
- Compares head vs base on PRs and fails when adoption drops below the
60% threshold.
- Writes backpack-adoption-results.json.
Consumers point Skyscanner/push-custom-cortex-data at the JSON file and
use "backpack-adoption" as the data-descriptor key (unchanged).
Changes:
- Delete src/cortex/ (upload-to-cortex.ts and its test).
- Drop cortex-webhook-uuid and cortex-entity inputs from action.yml.
- Remove cortex coupling from run.ts, summary.ts, types.ts.
- Rename BACKPACK_ADOPTION_CORTEX_KEY → BACKPACK_ADOPTION_OUTPUT_KEY.
- README: add Inputs table and a separate "Uploading metrics to Cortex"
section showing the push-custom-cortex-data step.
- Rebuild dist (12 → 8 tests, ~200 LOC removed from src and bundle).
Validation: lint, typecheck, test (8/8), build, verify-dist all pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The action and ds-analyser were producing different adoption percentages on the same repo. Root cause: this action ignored *.spec.*, *.stories.*, and mock directories by default, while ds-analyser's report-json.js only ignores node_modules, dist, build, and *.test.*. Mirror ds-analyser's exact ignore list so both tools count the same set of files and converge on the same numerator/denominator. Removed from defaults: **/*.spec.* **/*.stories.* **/__mocks__/** **/__mock__/** **/mocks/** Also normalises node_modules / dist / build patterns to drop the leading **/ prefix to match ds-analyser exactly. Test changes: split the ignore-behaviour test into two — one regression guard for what is still ignored, one positive assertion that spec/story/ mock files are now scanned (the new alignment behaviour). Validation: lint, typecheck, test (9/9), build all pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous classifier diverged from Skyscanner/ds-analyser on multiple
edge cases (className handling, classNames(variable), CSS module parsing,
non-Backpack visual classification), producing different adoption
percentages on the same repo.
Replace the classifier with a verbatim port of ds-analyser's
src/analyzer.js so the action's numbers match ds-analyser's
report-json.js output one-for-one. Helpers split by concern but each
mirrors ds-analyser's source line-by-line for easy auditing:
- jsx-helpers.ts — isBackpackComponent, isRawHTMLElement,
isNonBackpackComponent, isNonVisualComponent,
getJSXElementName, extractProps,
detectVariant, isDesignSystemImport
- css-helpers.ts — isCSSModuleImport, resolveCSSModulePath,
parseCSSModule (BEM-aware), CSS category
inference (analyzeCSSCategories,
analyzeCSSRulesCategories,
getCSSPropertyCategories)
- class-name.ts — extractClassNameInfo,
extractClassNamesFromFunction
(classNames(variable) → no override,
classNames('a','b') → overrideCount 2)
- visual-components.ts — buildVisualComponentRegistry,
checkBodyForVisualJSX
- analyze-repository.ts — analyzeFile + analyzeRepository, then a
thin AdoptionReport wrapper using
ds-analyser's pure / non-pure formula
(pureBackpackUsages = backpackUsages
- classNameOverrides)
Drops legacy jsx.ts and imports.ts; their helpers are folded into the
new files.
Tests: existing fixtures still pass (9/9 → 11). Two new parity tests
lock in the ds-analyser-specific behaviour:
- classNames(variable) produces hasOverride: false
- classNames('a','b','c') counts 3 overrides
Validation: lint, typecheck, test (11/11), build, verify-dist all pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
….json The previous alignment commit (06839a1) targeted ds-analyser's report-json.js defaults, but that file is a standalone debug script. The canonical adoption metrics shown on the dashboard / history.json are produced by seed-worker.js -> generateSimplifiedReport (report-json-api.js) with options.ignore from repos.json — every repo there configures the 9-pattern list with **/ prefixes. cli.js (`dsa analyze`) uses the same 9-pattern list as its CLI default. Re-align DEFAULT_IGNORE_PATTERNS to that canonical list so adoption percentages produced by this action match the published numbers: **/node_modules/**, **/dist/**, **/build/**, **/*.test.*, **/*.spec.*, **/*.stories.*, **/__mocks__/**, **/__mock__/**, **/mocks/** Notably, the missing **/ prefix on node_modules/dist/build let the analyser walk into nested per-package node_modules and dist directories in monorepos like Backpack itself, inflating the denominator. Also drop the `nodir: true` glob option to match ds-analyser's glob() call exactly (the **/*.{jsx,tsx} pattern can't match directories anyway, so this is a no-op for output but keeps the call site identical). Test: invert the spec/story/mock alignment test to assert these files are now ignored, and add coverage for `__mock__/` and `mocks/` paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Manual workflow that validates the action on a resolved commit SHA, then creates an immutable backpack-adoption-guard/v<version> tag and force-moves the floating backpack-adoption-guard/v<major> tag to the same commit. No GitHub Release is created so the existing release.yml and release-drafter flow for @skyscanner/backpack-web is unaffected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…errors Two correctness fixes for the guard, both reproducible by reviewers: 1) pull_request_target silently demoted the guard to "not_applicable". GitHub sets GITHUB_REF=refs/heads/<base> for pull_request_target events, so isMainBranch() returned true and run.ts skipped baseReport before evaluateGuard could see it. Result: branch.isPullRequest=true but guard.status=not_applicable on every PR-to-main run. Drop pull_request_target from isPullRequestEvent(). Consumers that need PR-level guarding must use pull_request; pull_request_target now falls through to the main reporting path, which is the safe behaviour given that pull_request_target's default checkout is the base branch (head == base, no meaningful comparison anyway). 2) Parse errors were treated as a warning while the offending file's element counts were silently dropped from the totals. A raw-HTML-heavy file failing to parse on head therefore inflated head's backpack percentage and could turn a real regression into a guard pass. evaluateGuard now refuses to evaluate when either head or base has parseErrors > 0 (status: fail, downgraded to warn under dry-run). Main-branch reporting paths are unaffected — they were already reporting-only and parse errors there don't gate merges. Tests: +4 parse-error cases (head fail, base fail, dry-run warn, main not_applicable). 15/15 pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…h base - Collapse GuardStatus to 3 values (pass/fail/warn); drop not_applicable. Main runs now report as pass (warn when files were skipped). PR runs that cannot load main fail (warn under dry-run) instead of silently passing. - Rewrite step summary with PR / main views: emoji header, plain-English one-liner, side-by-side comparison table on PRs, single-side table on main, collapsible parse-error file list, and Run details. Renames Head/Base/Delta to "This PR" / "Main" / "Change" so the audience does not need git jargon. - Reword guard reasons in natural English, using "this PR" and "main". - Auto-fetch the base ref before creating the worktree (try fetch-by-SHA, then fetch-by-branch, both with --depth=1). Consumers can now use the default shallow actions/checkout; fetch-depth: 0 becomes a fallback rather than a requirement. Failures are caught and surfaced via evaluateGuard. - Update tests: drop the old not_applicable assertions, add coverage for the new main+parseError=warn and PR+missingBase paths. 17/17 pass. - README: drop the fetch-depth: 0 from examples, document the 3 statuses.
…doption-action # Conflicts: # package-lock.json
- evaluate-guard: check threshold before parse errors so PRs are never blocked when main is below 60%, matching the README contract. Parse errors at/above the threshold still refuse to evaluate. - README + summary copy: align status and branch-context tables with the revised behaviour (parse errors flagged but non-blocking when below threshold; can fail when at/above threshold). - release workflow: drop the unused contents: write on the CreateTags job. Pushes already use the App token via http.extraHeader and persist-credentials is off, so the default GITHUB_TOKEN only needs contents: read for the checkout. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Provides a single-job reusable workflow that consumers can call with five
lines of YAML:
jobs:
backpack-adoption-guard:
uses: Skyscanner/backpack/.github/workflows/backpack-adoption-guard.yml@backpack-adoption-guard/v1
with:
cortex-entity: my-entity
secrets:
CORTEX_WEBHOOK_UUID: ${{ secrets.BACKPACK_ADOPTION_CORTEX_WEBHOOK_UUID }}
Behaviour:
- Checkout uses PR head SHA on pull_request events; falls back to
github.sha for push events.
- Runs the guard with dry-run defaulting to true (consumer-safe; can be
overridden). The action's own default of false is preserved for direct
action callers — see README "Manual setup".
- Uploads to Cortex via push-custom-cortex-data only on pushes to the
repository's default branch (auto-detected via github.ref_name).
- CORTEX_WEBHOOK_UUID is required even on PR runs because workflow_call
resolves secrets up front; it is not consumed unless the upload step
fires.
README rewritten to promote the reusable workflow as the recommended
setup and demote direct action invocation to an "advanced manual setup"
section, with both Cortex-upload variants documented.
Depends on the `backpack-adoption-guard/v1` tag created by the release
workflow once #4665 lands and is released. Until then this branch's
reusable workflow file references a tag that does not yet exist; CI
validates the YAML but nothing actually invokes it on this PR.
Co-Authored-By: Claude Opus 4.7 (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
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
.github/workflows/backpack-adoption-guard.ymlas a reusable workflow (workflow_call) that wraps checkout + the guard action + Cortex upload.github.event_name == 'push' && github.ref_name == github.event.repository.default_branch.Consumers go from ~26 lines of bespoke YAML (current carhire-homepage form) to:
Why a separate PR
This is stacked on top of #4665 because the reusable workflow internally references
Skyscanner/backpack/packages/backpack-adoption-guard@backpack-adoption-guard/v1— that tag only exists once #4665 merges and the release workflow is run.Recommended landing order:
Backpack Adoption Guard Releasewithversion=1.0.0to createbackpack-adoption-guard/v1.0.0andbackpack-adoption-guard/v1.@backpack-adoption-guard/v1directly to confirm v1 works end-to-end.main, merge, then bumpv1.1.0so consumers can opt into the reusable form.Design notes
dry-rundefault istruein the reusable workflow (vsfalseon the underlying action). Reusable workflow is consumer-facing and should be safe by default; consumers opt into enforcement explicitly. The README documents this deliberate divergence.CORTEX_WEBHOOK_UUIDis required even though PR runs don't consume it. This is intentional — it forces consumers to wire up Cortex up front, avoiding "half-installed" states.workflow_callresolves the secret eagerly regardless of whether the upload step runs.github.event.repository.default_branch, so this works for repos usingmain,master, or anything else.runs-onis exposed as an input with defaultubuntu-latest. Skyscanner repos that preferubuntu-latest-smallcan override.{}at workflow level andcontents: readat job level — the action itself only needs to read source.Test plan
dry-run: falsefrom a consumer surfaces a regression as a hard fail.🤖 Generated with Claude Code