Skip to content

Four checks — check:sdk:ts, check:sdk:python, verify-setup and check-webhooks-coverage — exist as scripts but run in no workflow #1247

Description

@Jagadeeshftw

Priority: Medium  ·  Difficulty: Medium  ·  Area: CI / repository guards  ·  Est. effort: 5–8 h

📌 Problem

package.json defines several verification scripts. None are referenced anywhere in .github/workflows/:

Script Purpose Refs in workflows
check:sdk:tsgenerate-sdk-ts.mjs --check Generated TS SDK matches the schema 0
check:sdk:pythongenerate-sdk-python.mjs --check Generated Python SDK matches the schema 0
verifyscripts/verify-setup.ts Environment/setup validation 0
test:coverage:ciscripts/check-webhooks-coverage.mjs Webhook coverage floor 0

The two SDK checks are the most consequential. A --check mode exists precisely so CI can assert that committed generated SDKs still match their source of truth. Because it never runs, the TypeScript and Python SDKs can drift from the API schema without anything noticing — and consumers of those SDKs get silently wrong types.

check-webhooks-coverage.mjs is reachable only through test:coverage:ci, which no workflow invokes, so the webhook coverage floor it enforces is currently not enforced at all. This matters more than usual here: webhook dispatch is the module with the SSRF surface.

This is a recurring pattern across this organisation — build the guard, test it, never switch it on.

🎯 Design decision required

State and defend:

  1. Blocking or advisory, per script. Run each first: some will fail on pre-existing drift. A guard that blocks on a large backlog gets disabled; one that only warns gets ignored. Decide per script and justify.
  2. SDK drift handling. If the generated SDKs are already out of date, is the fix to regenerate and commit them in this PR, or to land the check and fix drift separately? Either is defensible; say which and why.
  3. Placement. One combined "guards" job, or steps in the existing CI job? Consider whether SDK drift should block a merge that does not touch the schema.

🧩 Requirements and context

  • Run all four against the current tree first and paste the output. That inventory is the first deliverable.
  • Do not suppress a finding to get green. Fix it, or record it visibly with a stated reason.
  • verify-setup.ts may expect environment that CI does not have — establish what it needs before wiring it, and say so if it is not CI-appropriate.
  • Regenerating SDKs must be reproducible: the same input must produce the same output, or the check will be flaky. Verify this.
  • CI uses pnpm with --frozen-lockfile; keep that.

🛠️ Suggested execution

  1. Run all four locally; paste full output.
  2. Triage findings per your decision.
  3. Wire the appropriate scripts into CI.
  4. Prove each fires — introduce a deliberate violation per guard on a scratch branch and link the failing run.
  5. Report added CI runtime.

✅ Acceptance criteria

  • The PR opens with current output from all four scripts.
  • Each script is wired in, or explicitly justified as not CI-appropriate.
  • Each newly-wired guard is demonstrated failing on a deliberate violation (links).
  • Pre-existing findings are fixed or recorded visibly — none suppressed.
  • SDK generation is shown to be deterministic across two runs.
  • Blocking-vs-advisory is stated per script with reasoning.
  • pnpm typecheck, pnpm lint and pnpm test pass.

🚫 Out of scope

  • Redesigning what any script checks.
  • Rewriting the SDK generators.
  • Migration or lockfile issues — separate issues.

🧪 Verification

pnpm install --frozen-lockfile
pnpm check:sdk:ts
pnpm check:sdk:python
node scripts/check-webhooks-coverage.mjs
pnpm verify

📤 What your PR must include

  1. Full pre-change output of all four scripts.
  2. Blocking-vs-advisory decision per script.
  3. Your SDK-drift handling decision.
  4. Links to a failing run per newly-wired guard, plus runtime cost.
  5. Closes #<n>.

🔒 Security notes

The webhook coverage floor guards the module that performs server-side requests to user-supplied URLs — the SSRF surface. Leaving that floor unenforced means coverage there can silently erode, which is precisely where erosion is least acceptable. Separately, a generated SDK that has drifted from the schema can mislead consumers about which fields are optional or validated, pushing incorrect assumptions into downstream code.

📋 Guidelines

  • Minimum 95% test coverage on changed lines
  • Clear documentation
  • Timeframe: 96 hours from assignment
  • One logical change per commit; no merge commits

💬 Join our community

Working on this, or want to sanity-check your approach before you start? Come and ask — the maintainers are there and happy to help.

Telegram: https://t.me/Grainlify

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

GrantFox OSSGrantFox open-source programMaybe RewardedGrantFox: potentially rewarded contributionThird CampaignGrantFox third campaign issuecipriority:mediumMedium difficulty / self-contained but non-trivial

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions