o1-labs / o1-labs/Archive-Node-API

Check Schema and the npm audit gate are not required status checks, so neither can block a merge

Open
#213 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

P1 production-readiness
Dominant language
TypeScript
Stars
19
Forks
9
Avg merge
14h 20m
Merged PRs (30d)
8

Description

Problem

main's branch protection requires only two status checks:

required status checks : ["Run-Tests", "Linting"]
strict                 : true
required approvals     : 1
dismiss_stale_reviews  : true
enforce_admins         : false

Several CI jobs that exist specifically to prevent a class of breakage are therefore advisory: they go red without blocking a merge. The two that matter:

1. Check Schema (graphql-inspector) is not required

.github/workflows/graphql-inspector.yaml runs a breaking-change check with approve-label: expected-breaking-change, and the expected-breaking-change label already exists in the repo. But because the check is not required, a PR that removes or renames a schema field goes red and merges anyway.

This matters more than usual here because #198 introduces a versioning and schema-stability policy whose Enforcement section states that a breaking schema change "fails the build … so every breaking change is a deliberate, reviewed decision." That is currently not true. The policy leans on a gate that is not wired up.

It also matters downstream. mina-explorer-api classifies any response containing Cannot query field, Unknown argument, or Unknown type as a permanent schema error (app/upstream/graphql.py:33-42), which drives tier fallback and poisons its capability cache — so an accidental field removal does not merely fail, it fails stickily until that cache is cleared.

2. The new npm audit gate (#192) will not be required either

#192 adds .github/workflows/security.yaml with a real, correctly-calibrated gate (npm audit --omit=dev --audit-level=critical, no || true, no continue-on-error). Verified against main's lockfile it currently exits 0, so it is not a permanently-red gate. But as a non-required check it cannot block anything.

Proposed resolution

After #192 merges (so the check name exists and has at least one successful run on main), update branch protection:

gh api -X PATCH repos/o1-labs/Archive-Node-API/branches/main/protection/required_status_checks \
  -f 'contexts[]=Run-Tests' \
  -f 'contexts[]=Linting' \
  -f 'contexts[]=Check Schema' \
  -f 'contexts[]=npm audit'

Confirm the exact check names first — they must match the name: GitHub reports, not the workflow filename:

gh pr checks <any-recent-pr> --json name,bucket

Order matters: add a context only after that check has run successfully at least once on a PR against main. Adding a context that never reports leaves every PR permanently blocked on a pending check.

Do this after the current PR batch

See #211. Adding required checks mid-merge-train would force re-runs across ~17 open PRs. Sequence it after the batch lands.

Consider also
  • unit-tests is a separate job from Run-Tests and is not required. Worth adding — it is fast and it is what pins the downstream error-text contract (#195's error-masking.test.ts).
  • If Check Schema is made required, document in #198's policy that the escape hatch is the expected-breaking-change label, so the gate does not become something people route around by disabling it.

Acceptance criteria

  • Check Schema is a required status check on main
  • The npm audit gate from #192 is a required status check on main
  • A PR that removes a schema field is blocked from merging unless labelled expected-breaking-change
  • #198's Enforcement wording matches what is actually enforced

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Inspect .github/workflows/graphql-inspector.yaml and .github/workflows/security.yaml, then verify the reported check names with gh pr checks --json name,bucket. After #192 and the current PR batch land, update main branch protection to require Check Schema and npm audit, and confirm schema-breaking changes are blocked unless expected-breaking-change is applied and #198's wording matches.

Written by the indexing model from the issue text.

Assessment

Tech stack
github-actions, graphql, typescript
Domain
ci-cd, devops, security
Issue type
Feature
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.