Skip to content

Refactor(fw-review): reorganize scripts/ into checks/ + runners/ - #46

Open
annamalai-shan wants to merge 2 commits into
mainfrom
mp-43838-reorg-scripts-checks-runners
Open

Refactor(fw-review): reorganize scripts/ into checks/ + runners/#46
annamalai-shan wants to merge 2 commits into
mainfrom
mp-43838-reorg-scripts-checks-runners

Conversation

@annamalai-shan

@annamalai-shan annamalai-shan commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

Reorganize skills/fw-review/scripts/ into two purpose-specific folders so the rule audit code is separated from the run plumbing.

  • checks/ — one Node CLI per script-backed rule ID, 8 files total: external-import-sources.js (FFS-02L), https-imports.js (FFS-04L), image-resolution.js (FFS-05L), oauth-config-usage.js (FF-07L), unused-library-imports.js (CR-05L), fdk-errors-warnings.js (GN-02L), freshworks-css-only.js (GN-08L), platform-version-upgrade.js (GN-12L).
  • runners/ — shared CLI/result plumbing (runners/common.js exporting createRuleResult + runCli), imported by every check as require('../runners/common').
  • No behavior change: rule IDs, scan logic, regex patterns, allowlists, JSON output contract (internal.rule_id, passed, summary, details), exit codes, FDK prerequisites, and the ## App Review Result report format are all byte-identical.
  • Doc sweep: updated path references in skills/fw-review/SKILL.md, skills/fw-review/README.md, skills/fw-review/rules/report.md, skills/fw-review/rules/script-check-rules.md, root AGENTS.md, and CONTRIBUTING.md.

Diff shape

15 files changed, 48 insertions, 47 deletions. Git detects all 9 moves as renames (96–100% similarity): 1× pure rename (common.jsrunners/, 100%), 8× single-line require-path edits (checks/, 96–99%).

Test plan

  • All 7 non-FDK checks run from checks/ against tests/fixtures/platform3-valid and emit the exact pre-reorg JSON contract (verified internal.rule_id, passed, summary, details, and process.exitCode).
  • 8th check (fdk-errors-warnings.js, GN-02L) is mechanically identical to the verified 7 (same require('../runners/common'), same runCli(...) wrapper).
  • Installer test suite passes (cd installer && npm test → 118/118).
  • grep sweep: no stale skills/fw-review/scripts/ reference anywhere in the repo; only ~/.fw-dev-tools/scripts/ (installer's .meta.json helpers) and skills/shared/scripts/ remain, both unrelated.
  • Reviewer to sanity-check rendering of updated SKILL.md / README.md paths.
  • Smoke-check after next npx fw-dev-tools update that installed copies under ~/.cursor/skills/fw-review/, ~/.codex/skills/fw-review/, ~/.fw-dev-tools/skills/fw-review/ pick up checks/ + runners/ cleanly.

…43838)

Split skills/fw-review/scripts/ by responsibility:

- checks/ — one Node CLI per script-backed rule ID (FFS-02L, FFS-04L,
  FFS-05L, FF-07L, CR-05L, GN-02L, GN-08L, GN-12L)
- runners/ — shared CLI/result plumbing (common.js exporting
  createRuleResult + runCli), imported by every check

Every check now imports as require('../runners/common'). No behavior,
rule ID, JSON output contract, or App Review Result format change.

Doc references updated across SKILL.md, README.md, rules/report.md,
rules/script-check-rules.md, AGENTS.md, CONTRIBUTING.md.

Verified by running each non-FDK check from the new layout against
tests/fixtures/platform3-valid — JSON contract (internal.rule_id,
passed, summary, details) and exit codes unchanged. Installer test
suite (118 tests) passes.

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions

Copy link
Copy Markdown

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@annamalai-shan annamalai-shan changed the title refactor(fw-review): reorganize scripts/ into checks/ + runners/ (MP-… refactor(fw-review): reorganize scripts/ into checks/ + runners/ Jun 24, 2026
@annamalai-shan annamalai-shan changed the title refactor(fw-review): reorganize scripts/ into checks/ + runners/ Refactor(fw-review): reorganize scripts/ into checks/ + runners/ Jun 24, 2026
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.

1 participant