Skip to content

ci: add PR checks for develop (CI/CD Phase 1, PR 2 of 4) - #118

Merged
umermjd11 merged 1 commit into
InfiniteZeroFoundation:developfrom
Santiagocetran:ci/phase1-workflow
Sep 10, 2026
Merged

umermjd11 merged 1 commit into
InfiniteZeroFoundation:developfrom
Santiagocetran:ci/phase1-workflow

Conversation

@Santiagocetran

@Santiagocetran Santiagocetran commented Sep 4, 2026 •

Copy link
Copy Markdown
Collaborator

Important

Blocked on #117 — please merge that first. This PR touches no files under foundry/, but develop currently fails 3 of 240 forge test cases, so this PR's merge commit inherits them and the Solidity job goes red. #117 fixes exactly those three. Draft until it lands.

PR 2 of 4 for CI/CD Phase 1, per discussion #74. PR 1 (#103) made the tree CI-ready; this turns the gate on.

What runs

One workflow on pull_request → develop, three jobs plus a CI OK summary job:

Job Blocking Today
Solidity — forge build/test, hardhat compile/test yes 241 + 32 (once #117 lands)
Python — pytest -m "not integration" yes 298 passed, 133 deselected
Docs — relative-link check over Documentation/ yes 154 links, 0 broken
forge lint advisory 37 findings
ruff advisory 128 under the proposed gate set

CI OK is meant to be the single required status check. One context keeps branch protection simple and avoids the trap where path-filtered jobs are marked required — a docs-only PR would otherwise wait forever on Solidity checks that never report.

The two lint checks ship advisory on purpose. Making them blocking today would fail every PR on a pre-existing backlog and teach people to fight CI. They get promoted in PR 3, once the backlog is cleared.

About that red check

The Solidity job will be red until #117 merges, and it's worth reading rather than apologising for: the gate's very first run catches three tests that have been broken on develop since #66 and passed human review. That's the case for having a gate, made by the gate.

Python and Docs are green on the merge commit right now — I ran every job's steps locally against this branch rebased onto develop. So the red is isolated and legible, not ambiguous.

Heads-up for other contributors

Once this lands on develop, open PRs targeting develop start getting gated on their next push — currently #116, #110, #109, #77. Some may go red on the advisory-free blocking checks. That's the intended effect, but nobody should be surprised by it. PRs targeting feature branches (#72, #32, #31, #29, #27) are unaffected, since the trigger is branches: [develop] only.

A few choices worth flagging

Pinned, not floating. The Foundry release (v1.7.1), the action SHAs, and the Python test versions (.github/constraints-ci.txt) are all pinned. version: stable would let an upstream release change compilation or lint output on an unrelated PR.

npm ci in foundry/ before forge test. Without it, Upgrades.validateImplementation's parallel FFI calls to npx @openzeppelin/upgrades-core race each other. Reproducible failure, and it also stops a test fetching a package at test time — which matters because foundry.toml sets ffi = true.

forge lint goes through a parser, not a grep. .github/scripts/forge_lint_gate.py validates forge's diagnostic schema, so an unrecognised record type or a diagnostic missing its severity fails closed rather than being silently skipped. Two traps made this necessary: --json writes diagnostics to stderr, and it can't be combined with --color (which exits 2 emitting nothing).

No secrets are used or needed, so fork PRs work and are safe by default. main is untouched.

tests/test_ci_scripts.py covers both helper scripts (21 tests): the link checker's fence tracking and the lint gate's fail-closed schema validation.

Three things I need from @umermjd11

  1. Please approve the workflow run on this PR. No checks have appeared, and I'm fairly sure that's because my access here is triage, not write — GitHub holds workflow runs from fork PRs by non-write contributors until a maintainer clicks Approve and run workflows. I can't confirm it directly (runs in action_required aren't visible without write access), so if you look and there's genuinely nothing queued, tell me and I'll dig further.
  2. Can you set branch protection on develop? This PR is worth much less without it — the plan uses a single CI OK context. I don't have admin.
  3. Is Actions enabled? Answered while writing this — yes, 248 runs. The four active workflows are all GitHub-managed (Dependabot, Dependency Graph, CodeQL default setup); there's no user-defined workflow file on any branch, so this is genuinely the first one.

Implements the workflow proposed in discussion InfiniteZeroFoundation#74. Runs on `pull_request`
targeting `develop`, on open and on every push to the PR.

Three jobs, plus a `CI OK` summary job intended to be the single required
status check. One context keeps branch protection simple and avoids the trap
where path-filtered jobs are marked required: a docs-only PR would wait forever
on Solidity checks that never report.

Blocking today: forge build/test (241 tests), hardhat compile/test (32),
pytest -m "not integration" (298), and a relative-link check over
Documentation/ (154 links).

Advisory today: `forge lint` and `ruff`. Both are real but carry a backlog —
37 Solidity findings and 128 Python ones under the proposed gate set — and a
gate that fails on pre-existing findings teaches contributors to fight CI.
They run with continue-on-error and get promoted in a follow-up once the
backlog is cleared.

`forge lint` is gated through .github/scripts/forge_lint_gate.py rather than by
scraping human-readable output. Two traps make that necessary: `--json` writes
diagnostics to stderr, and it cannot be combined with `--color` (which exits 2
emitting nothing). The parser validates forge's diagnostic schema, not just its
JSON syntax, so an unrecognised record type or a diagnostic with a missing
severity fails rather than being silently skipped.

Pinned deliberately: the Foundry release (v1.7.1 — the toolchain every
measurement was taken against, and whose output the lint gate parses), the
action SHAs, and the Python test versions via .github/constraints-ci.txt.
`version: stable` would let an upstream release change compilation or lint
output on an unrelated PR.

`npm ci` runs in foundry/ before `forge test`, without which
Upgrades.validateImplementation's parallel FFI calls to
npx @openzeppelin/upgrades-core race each other. It also stops a test from
fetching a package at test time, which matters because foundry.toml sets
ffi = true.

`defaults.run.shell: bash` is explicit rather than assumed: ci-ok's result loop
relies on word splitting, which zsh and dash do not perform.

.gitignore gains a general `__pycache__/` rule — .github/scripts/ is imported
by tests, which was not a bytecode site before.

No secrets are used or needed, so PRs from forks work and are safe by default.
`main` is untouched.

tests/test_ci_scripts.py covers both helper scripts (21 tests): the link
checker's fence tracking and the lint gate's fail-closed schema validation.

Verified locally against this branch rebased onto develop, by running every
job's steps in order: hardhat green (32), python green (298 passed, 133
deselected), docs green (154 links, 0 broken). The advisory lint steps report
their backlog without failing their jobs.

The solidity job is RED until InfiniteZeroFoundation#117 merges, and not for a reason in this PR.
develop currently fails 3 of 240 forge tests in SlashingInvariants.t.sol --
stale assertions against the pre-InfiniteZeroFoundation#65 slash() semantics. This PR touches no
files under foundry/, so its merge commit inherits that failure verbatim.
InfiniteZeroFoundation#117 fixes it (241 passed); this PR must not merge before it.

That first red run is worth reading rather than apologising for: the gate's
very first execution catches three tests that have been broken on develop
since InfiniteZeroFoundation#66 and passed human review. That is the case for the gate, made by
the gate.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Wu3sbCps6PEhyvsQNGK3pr
@umermjd11
umermjd11 marked this pull request as ready for review September 10, 2026 01:05
@umermjd11

Copy link
Copy Markdown
Collaborator

Reviewed in an isolated worktree, branch head 0e3b6de, against current develop HEAD 890626b (which already includes #117 — the fix this PR was blocked on). Branch was cut from 3c1f100, and none of the 6 files this PR touches were changed by develop since. GitHub agrees: mergeable: MERGEABLE, mergeStateStatus: CLEAN (confirmed with a local git merge-tree dry run: clean, no conflicts). This is CI/CD Phase 1 PR 2 of 4 (discussion #74), turning on the gate that #103 (PR 1) prepared for. Ran every job's real steps rather than just reading the workflow YAML — including a genuine trial merge onto current develop to check the one thing the PR's own description couldn't verify (that #117 landing actually turns the Solidity job green).

Claimed: the Solidity job is red on this PR's own branch (pre-#117), but that's not a defect in this PR — it's inherited from develop's then-current state, and resolves once #117 merges.

Verified — and independently checked the part the PR body couldn't (#117 wasn't merged yet when it was written): ran forge test on this PR's isolated branch — 237 passed, 3 failed, the exact pre-#117 SlashingInvariants.t.sol failures. Then did a real trial merge of this PR onto current develop (which now has #117) and ran forge test there — 254/254 pass. The Solidity job genuinely goes green once this merges now; the blocker is resolved.

Claimed: all 4 pinned Action SHAs correspond to the versions named in their comments (actions/checkout@... # v7.0.1, actions/setup-node@... # v7.0.0, actions/setup-python@... # v7.0.0, foundry-rs/foundry-toolchain@... # v1.9.1), and Foundry itself is pinned to v1.7.1.

Verified exactly as claimed — queried each action's repo directly (gh api repos/<action>/commits/<tag>) and confirmed every pinned SHA resolves to exactly the tag named in its comment. forge --version in the CI environment reports 1.7.1, matching FOUNDRY_VERSION.

Claimed: forge lint --json writes diagnostics to stderr, not stdout, and cannot be combined with --color (exits 2, emits nothing) — the two traps that make forge_lint_gate.py necessary rather than a plain grep.

Verified exactly as claimed — ran forge lint --json --color always directly: fails immediately with error: the argument '--json' cannot be used with '--color <COLOR>'. Ran forge lint --json alone: 0 bytes to stdout, 60,059 bytes to stderr.

Claimed: forge_lint_gate.py validates forge's diagnostic schema and fails closed on an unrecognised $message_type, a missing level, or an unknown severity — not just JSON syntax.

Verified exactly as claimed, all four failure modes reproduced directly: piped synthetic records at the script — an unrecognised $message_type fails with "forge's lint output has changed"; a diagnostic missing level fails with "diagnostic has no level"; an unknown severity ("catastrophic") fails with "classify it in FAIL_LEVELS or PASS_LEVELS"; empty input correctly passes as "0 diagnostic(s)". Then ran it against real forge lint --json output on the current tree — correctly parsed and reported 37 findings (31 unsafe-typecast, 6 block-timestamp), exact match to the PR's claimed count.

Claimed: the doc-link checker finds 154 relative links under Documentation/, 0 broken; its fence-tracking correctly handles nested/mismatched fence characters and lengths.

Verified exactly as claimed — ran check_doc_links.py Documentation directly: 154 checked, 0 broken. Read iter_prose's fence logic directly: tracks both the fence character and run length, so a closing fence must match both, which correctly prevents a ~~~ block containing a literal triple-backtick from toggling state early — a real bug class this specifically guards against.

Claimed: tests/test_ci_scripts.py — 21 tests covering both scripts.

Verified exactly as claimed — ran it directly: 21/21 pass, covering exactly the schema-fail-closed cases and fence-tracking edge cases described above.

Claimed: Python job — 298 passed, 133 deselected; hardhat job — 32 passing; both green on the merge commit today.

Verified exactly as claimed — pytest -m "not integration" -q on this PR's tree: 298 passed, 133 deselected. npx hardhat compile && npx hardhat test: 32 passing.

Claimed: no secrets are used or needed, so fork PRs work safely by default; main is untouched.

Verified — grepped the entire workflow for secrets.: zero matches. permissions: contents: read is the only top-level permission granted. The trigger is pull_request: branches: [develop] only, so main genuinely cannot be affected by this workflow.

Not independently re-verified: the actual GitHub Actions run itself (this review was static/local execution of each job's steps, not a live Actions run) — which is exactly item 1 of the PR's own "three things I need" list, addressed separately below.


Every checkable claim held up, including the one this PR's own description couldn't verify yet (that the Solidity job is actually green once #117 lands — now confirmed directly with a real trial merge onto current develop: 254/254). The lint gate's fail-closed design is real and independently reproduced against both synthetic schema violations and genuine forge lint output. This is an unusually rigorous CI PR — mergeable as-is.

@umermjd11

Copy link
Copy Markdown
Collaborator

Files changed (6) — as of 0e3b6de (PR head)

Diffed against merge-base 3c1f100 (develop) — branch cut recently, and none of this PR's 6 files were touched by develop since. GitHub agrees: mergeable: MERGEABLE, mergeStateStatus: CLEAN. Confirmed with a local git merge-tree dry run: clean, no conflicts. Also did a real trial merge onto current develop (which now has #117, this PR's stated blocker) and ran forge test there: 254/254 pass — the Solidity job is genuinely green once this merges.

This is CI/CD Phase 1 PR 2 of 4 (discussion #74); PR 1 (#103) prepared the tree, this turns the gate on.

(Table below is per-file, key/value, following the format from PR #63's review.)

.github/workflows/ci.yml

Field Value
Change New
Lines +163
Diff (what exactly is in this PR) New pull_request: branches: [develop] workflow: solidity job (forge build/test, npm ci first, advisory forge lint via the gate script, then hardhat compile/test), python job (pytest -m "not integration", advisory ruff), docs job (relative-link check), and a ci-ok summary job that fails if any of the three didn't succeed — intended as the single required status check.
Functionality — how & why How: all 4 third-party actions are pinned to full commit SHAs (verified each resolves to the exact tag named in its comment: checkout v7.0.1, setup-node v7.0.0, setup-python v7.0.0, foundry-toolchain v1.9.1), Foundry itself pinned to v1.7.1 via FOUNDRY_VERSION rather than stable. npm ci in foundry/ runs before forge test specifically because Upgrades.validateImplementation shells out over FFI (foundry.toml has ffi = true, confirmed) to npx @openzeppelin/upgrades-core, and parallel test execution racing an unpinned fetch is a real, reproducible failure mode. permissions: contents: read only, zero secrets.* references anywhere — fork PRs run safely with no maintainer-side secret exposure. Why: ci-ok as the single required context avoids the path-filter/required-check deadlock where a docs-only PR waits forever on a Solidity job that path-filtering never triggers. Pinning everything (actions, Foundry, PIP_CONSTRAINT) means an upstream release can't silently change compilation or lint output on an unrelated PR.
Diff vs current develop HEAD None — untouched since the merge-base; net-new file.
Recommended merge proposal Merged as-is. Independently verified all 4 action SHAs against their claimed tags via the GitHub API, confirmed ffi = true and zero secrets usage by reading the files directly, and — since this PR's own body couldn't verify it yet (written before #117 merged) — did a real trial merge onto current develop and ran forge test: 254/254 pass, confirming the Solidity job is actually green post-merge.
Actual merge proposal Soon
Pending proposal See the separate comment on the three asks from the PR body (workflow-run approval, branch protection).
Local merge conflict No
GitHub merge conflict No

.github/scripts/forge_lint_gate.py

Field Value
Change New
Lines +119
Diff (what exactly is in this PR) Parses forge lint --json's newline-delimited JSON diagnostics on stdin and fails closed: an unparseable line, an unrecognised $message_type, a diagnostic missing level/code, or an unknown severity all raise SchemaError and exit 1 — not just a warning/error finding itself.
Functionality — how & why How: independently reproduced all four fail-closed paths with synthetic input (unrecognised message type, missing level, unknown severity, unparseable JSON — each fails with a specific diagnostic message), confirmed empty input passes cleanly, and ran it against real forge lint --json output on the current tree — correctly reports 37 findings (31 unsafe-typecast, 6 block-timestamp), an exact match to the count independently confirmed in #117's review. Why: forge lint --json can't be combined with --color (confirmed: exits 2, "cannot be used with") and writes to stderr not stdout (confirmed: 0 bytes stdout, 60,059 bytes stderr) — a plain grep over human-readable output would be fragile to either trap; validating the schema itself means a future Foundry release changing the diagnostic shape fails the gate loudly instead of silently passing nothing.
Diff vs current develop HEAD None — untouched since the merge-base; net-new file.
Recommended merge proposal Merged as-is. All fail-closed paths and the real 37-finding count independently reproduced.
Actual merge proposal Soon
Pending proposal None
Local merge conflict No
GitHub merge conflict No

.github/scripts/check_doc_links.py

Field Value
Change New
Lines +89
Diff (what exactly is in this PR) Checks inline relative Markdown links under Documentation/, skipping fenced code blocks and external/anchor-only links. Docstring explicitly states what it does not handle (reference-style links, multi-line destinations, balanced-paren destinations, percent-encoding) and confirms none of those forms exist in the current corpus.
Functionality — how & why How: iter_prose's fence tracking records both the opening fence character and run length, so a closer must match both — read this directly and confirmed it correctly prevents a ~~~ block containing a literal triple-backtick from toggling fence state early, a real bug class a naive single-character check would miss. Ran it directly: 154 links checked, 0 broken, exact match to the PR's claim. Why: a docs-only PR needs a real, cheap, false-positive-free check to be part of the blocking docs job; the honestly-scoped docstring means the check's limits are documented rather than silently assumed away.
Diff vs current develop HEAD None — untouched since the merge-base; net-new file.
Recommended merge proposal Merged as-is. Fence-tracking logic and the 154/0 count both independently verified.
Actual merge proposal Soon
Pending proposal None
Local merge conflict No
GitHub merge conflict No

tests/test_ci_scripts.py

Field Value
Change New
Lines +153
Diff (what exactly is in this PR) 21 tests covering both helper scripts: the lint gate's fail-closed schema validation (unknown message type, missing level, unknown severity, missing code, unparseable/non-object JSON) and the doc-link checker's fence-tracking edge cases (backtick vs. tilde fences, longer/shorter runs, indented fences, unterminated fences, closing fences with an info string).
Functionality — how & why How/why: directly exercises the exact edge cases both scripts' docstrings claim to handle, at the unit level rather than only through the real forge lint/doc-corpus runs. Ran directly: 21/21 pass.
Diff vs current develop HEAD None — untouched since the merge-base; net-new file.
Recommended merge proposal Merged as-is.
Actual merge proposal Soon
Pending proposal None
Local merge conflict No
GitHub merge conflict No

.github/constraints-ci.txt

Field Value
Change New
Lines +3
Diff (what exactly is in this PR) Pins torch==2.6.0, pytest==9.1.1, ruff==0.16.4, applied via the workflow's PIP_CONSTRAINT env var.
Functionality — how & why How/why: keeps the Python job's dependency versions fixed the same way the Solidity job pins Foundry and action SHAs — an unrelated PR shouldn't see a different test/lint outcome because an upstream package released overnight.
Diff vs current develop HEAD None — untouched since the merge-base; net-new file.
Recommended merge proposal Merged as-is.
Actual merge proposal Soon
Pending proposal None
Local merge conflict No
GitHub merge conflict No

.gitignore

Field Value
Change Modified
Lines +5
Diff (what exactly is in this PR) Adds a general __pycache__/ rule, since .github/scripts/ is now imported by tests and wasn't a bytecode site before.
Functionality — how & why How/why: pure hygiene addition, comment explicitly notes the existing path-specific rules are now redundant but left in place to keep the diff small.
Diff vs current develop HEAD None — untouched since the merge-base.
Recommended merge proposal Merged as-is.
Actual merge proposal Soon
Pending proposal None
Local merge conflict No
GitHub merge conflict No

Verification

Independently reproduced every real-execution claim in the PR body: forge lint gate against real output (37 findings, exact match) and against 4 synthetic schema-violation cases; doc-link checker (154/0, exact match); test_ci_scripts.py (21/21); pytest -m "not integration" (298 passed, 133 deselected, exact match); hardhat test (32 passing, exact match); all 4 pinned action SHAs verified against their tags via the GitHub API. Went one step further than the PR body could at time of writing: did a real trial merge onto current develop (which now has #117) and ran forge test there — 254/254 pass, confirming the Solidity job's red state was genuinely inherited and is now resolved. Full detail in the deep-verification comment above.

Local vs. GitHub agree: both report a clean, conflict-free merge.

@umermjd11

umermjd11 commented Sep 10, 2026 •

Copy link
Copy Markdown
Collaborator

Update (2026-09-10): PR #118 has since merged to develop as 3e71ecb — actual outcome. ci.yml now exists on develop. Branch protection (item 2) is being set up now — full step-by-step procedure added under that section below. The original three-item check from before the merge is kept as-is underneath for the record.


@umermjd11 — checked all three asks directly against the repo's current state (my token has push but not admin/maintain, so some of this I can confirm and some I can't, noted below):

1. Workflow-run approval — likely needed, but I can't fully confirm it either

Queried the Actions API for this PR's exact head commit (0e3b6de): 0 runs found, and .github/workflows/ci.yml doesn't appear in the repo's registered workflows list at all (only the 4 GitHub-managed ones show up — see #3). That's consistent with Santiago's theory — a held action_required run for a first-time/triage-level fork contributor can be invisible even at push-level access, not just to someone with no access at all — but I can't rule out "nothing was queued yet" either from where I sit. You have more visibility than either of us here.

What to check: open the PR's Checks tab, or the repo's Actions tab, in the browser. If you see a banner or a run sitting in "action required," click Approve and run workflows. If you see genuinely nothing queued, that's new information — worth a comment on the PR either way so Santiago isn't stuck guessing.

Update: moot for this PR specifically now that it's merged — there's nothing left to approve on #118 itself. Still relevant going forward: .github/workflows/ci.yml is still not in the repo's registered workflows list and develop's tip has 0 check-runs, because the workflow only triggers on pull_request events and my merge was a direct git push (no PR-merge event fired). The next real PR opened against develop will be the first thing to actually trigger it — worth watching whether that run needs approval too, depending on the opening contributor's permission level and the repo's fork-PR-workflow-approval setting.

2. Branch protection on develop — confirmed: none exists today

GET /branches/develop/protection returns a plain 404 — there is no branch protection rule on develop at all right now. This needs your admin access; my token can't set it (admin: false).

What to set, per the PR's own design (a single required context to avoid the path-filter deadlock it describes):

  • Settings → Branches → Add branch protection rule → branch name pattern develop
  • Enable Require status checks to pass before merging
  • Search for and select CI OK (the exact job name from ci-ok in ci.yml) — not the three individual job names, since those are path-agnostic today but the PR's whole point is collapsing them into one context
  • Whether to also require branches to be up to date before merging, require PR reviews, etc. is a separate policy call outside what this PR asks for — worth deciding deliberately rather than defaulting

One consequence worth being deliberate about before you flip this on: per the PR body, open PRs targeting develop (currently #116, #110, #109, #77 per Santiago's list — #116 has since merged) start getting gated on their next push once this workflow exists on develop, and some may go red on the two blocking checks (Solidity/Python/Docs — the two lint checks are advisory only). That's the intended effect, not a bug, but it's easier to reason about if you turn on branch protection only after you've decided you're fine with that gating taking effect.

Update — full procedure, now that you're actually in Settings → Branches:

Step 0: get one green CI run first (blocks Step 4 otherwise)

Confirmed via the API: ci.yml still isn't in the repo's registered workflows list, and develop's tip has 0 check-runs — the workflow has never executed. It only triggers on pull_request events, and the merge that landed it was a direct git push, which doesn't fire one. GitHub's required-status-check search box only lists checks that have reported at least once in roughly the last week, so CI OK will not be selectable until then.

  • Open any PR against develop — a real one if one's ready, otherwise a one-line throwaway (e.g. a comment tweak) works fine and can be closed after.
  • Watch the PR's Checks tab. If the run sits in action required, click Approve and run workflows (this is the same hold flagged in item 1 above — expect it for lower-permission/first-time contributors, not for your own PRs).
  • Once all three jobs (Solidity, Python, Docs) and the ci-ok summary job finish, CI OK will show up as a check on that PR and become searchable in branch-protection status-check pickers repo-wide.

Step 1: choose the rule type

Click Add classic branch protection rule (not "Add branch ruleset"). Classic rules do exactly what's needed here — one branch, one required context — and match the design already written into the PR. Rulesets are the newer, more flexible replacement but add layering/bypass-list complexity this doesn't need; no reason to take that on for this.

Step 2: branch name pattern

Enter develop exactly (not a glob like develop*, unless you specifically want it to also match future branches with that prefix — it doesn't need to for this).

Step 3: enable "Require status checks to pass before merging"

Check the box. A search field appears underneath once it's checked.

Step 4: select the required check

Search CI OK and select it (only appears if Step 0 has completed). Select only CI OK — do not also add Solidity, Python, or Docs individually. That's deliberate: those three are path-agnostic today, and the whole point of the ci-ok summary job is collapsing them into one required context so a docs-only PR doesn't wait forever on a Solidity check that never reports. Adding the individual jobs as required alongside CI OK would reintroduce exactly the deadlock the PR was designed to avoid.

Optional, in the same section: "Require branches to be up to date before merging" — forces a PR to be rebased/merged with latest develop before its checks count as passing. Reasonable to enable given how much other work is landing on develop right now, but it does mean a PR can go from green to "needs update" just from develop moving, and every such update re-runs the full ~20 min Solidity job. Your call, not something #118 explicitly asks for.

Step 5: leave "Require a pull request before merging" OFF (for now)

Do not enable this unless you're intentionally changing how we merge. It blocks direct git push to develop entirely, including the exact workflow used to land this PR and the ones around it this session (local git merge --no-ff + git push origin develop, not GitHub's merge button). "Require status checks to pass before merging" alone only gates merges done through GitHub's own merge button — it does not restrict direct pushes on its own, so the two settings are independent. If you do want to lock down direct pushes going forward, that's a real, separate decision worth its own conversation (it'd mean every future merge — including ones like this session's — goes through GitHub PRs instead of local merge+push).

Step 6: save, then verify

Click Create (or Save changes). Confirm it took by re-running (or having me re-run) GET /repos/InfiniteZeroFoundation/DevNet/branches/develop/protection — it should return the rule instead of a 404.

Step 7: mind future open PRs this will start gating

Per the PR body, any PR open against develop gets gated on its next push once this is live — checked just now, and #116, #110, #109, and #77 (the four named in the PR body) have all since merged, so there's currently nothing open against develop that this would immediately hit. Still worth knowing for whatever opens next: some may go red on the blocking checks (Solidity/Python/Docs; the two lint checks stay advisory). That's the intended effect, not a bug — just worth knowing so nobody's surprised by a sudden red X on an otherwise-fine PR.

3. Actions enabled — already resolved, nothing for you to do

Independently confirmed via the API: exactly 4 workflows are registered (Dependabot Updates, Dependency Graph, CodeQL, CodeQL - Code Quality), all GitHub-managed, none user-defined — matches Santiago's own answer in the PR body exactly. This genuinely is the first user-defined workflow in the repo.


Net: item 3 is done, item 2 is a real action only you can take (and worth pairing with a decision on when to let the new gating hit those 4 open PRs), item 1 needs your eyes in the browser since neither Santiago nor I can see far enough into action_required state to confirm it from here.

@umermjd11
umermjd11 merged commit 3e71ecb into InfiniteZeroFoundation:develop Sep 10, 2026
@umermjd11

Copy link
Copy Markdown
Collaborator

Actual outcome — PR #118 merged (pushed)

One commit, on origin/develop:

  1. 3e71ecb — real merge of this PR (merge commit, not squash), parents eb11afe (prior develop tip) and 0e3b6de (this PR's head, unchanged). No conflicts — GitHub agrees: PR shows MERGED.

Files unchanged from the PR (6 of 6)

.github/constraints-ci.txt, .github/scripts/check_doc_links.py, .github/scripts/forge_lint_gate.py, .github/workflows/ci.yml, .gitignore, tests/test_ci_scripts.py — all landed byte-for-byte as authored. This PR's own text flagged it as blocked on #117 (which merged earlier today as 890626b, fixing the SlashingInvariants.t.sol failures this PR would otherwise have inherited into a red Solidity job) — confirmed that's no longer an issue: the branch was cut Aug 26 but develop's Solidity suite is green now.

Verification (against the actually-pushed state, not just the PR's reported numbers)

  • Local merge clean, zero conflicts, matching GitHub's mergeable: MERGEABLE / mergeStateStatus: CLEAN.
  • tests/test_ci_scripts.py re-run on the real merge commit: 21/21 pass.
  • .github/scripts/check_doc_links.py run directly against current Documentation/: 154 inline relative links, 0 broken — confirms the checker still holds against docs that have moved on since this branch was cut.
  • Full pytest -m "not integration" re-run on both the merged tree and plain pre-merge develop: identical pre-existing 7 collection errors / 4 failures in both cases (missing torch/eth-account/py-multibase in this local venv — a known, tracked environment gap, not something this PR touches or regresses).
  • Read .github/workflows/ci.yml end to end: pinned action SHAs and Foundry version, permissions: contents: read only, no secrets referenced — matches the PR's own "no secrets needed, fork PRs work" claim.

Local vs. GitHub agree: clean merge as predicted, no surprises.

Not done as part of this merge

The PR's two operational asks — approving the currently-held workflow run, and turning on branch protection with CI OK as the single required status check — are maintainer/admin actions outside the scope of merging the file itself, and still need doing separately for the gate to actually take effect on new PRs.

@umermjd11

Copy link
Copy Markdown
Collaborator

Status update — both pending items resolved.

1. Workflow-run approval — resolved, nothing further needed

Confirmed via the Actions API: ci.yml has been running normally on every pull_request event since 2026-09-10, across all the PRs currently open against develop (#140–142, #146–149) — a mix of legitimate successes and failures, no action_required holds visible anywhere in the run history. Whatever triage-level hold Santiago's theory pointed at either never materialized here or cleared on its own; no manual approval step has been needed.

2. Branch protection on develop — confirmed live

GET /repos/.../branches/develop now reports "protected": true, and Umer confirms Settings → Branches shows a develop rule applying to 1 branch. (Note for anyone re-checking via API: the detailed /branches/develop/protection endpoint 404s for any token without admin on the repo — mine included — even though the rule exists; the plain branch object's protected flag is the reliable signal at push-level access.)

Net: both open items from the earlier update are done. Nothing further outstanding from this thread.

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.

2 participants