Skip to content

Wire pre-commit into CI, fix triggers that never ran, fix gates that never blocked#126

Draft
cryptoxdog wants to merge 9 commits into
cursor/enable-ci-autofix-ddaffrom
cursor/repair-ci-pipeline-precommit-ddaf
Draft

Wire pre-commit into CI, fix triggers that never ran, fix gates that never blocked#126
cryptoxdog wants to merge 9 commits into
cursor/enable-ci-autofix-ddaffrom
cursor/repair-ci-pipeline-precommit-ddaf

Conversation

@cryptoxdog

@cryptoxdog cryptoxdog commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator

Summary

Narrowly scoped to the CI pipeline itself (no lint/style remediation here — that's #125, which this branch is stacked on top of so the new checks below can actually run green). Addresses: "ci scripts/tests must run at pre-commit, wired that way so errors are caught before they hit PRs or the codebase" + "CI should surface every error, not just one, and PRs with CI failures should be blocked."

0. CRITICAL: ci.yml (and 4 other workflows) have never actually triggered

While verifying this PR against live GitHub Actions, gh run list showed ci.yml (and docker-build.yml, supply-chain.yml, codeql.yml, release-drafter.yml) failing at 0 seconds on every push, with 0 jobs created — "This run likely failed because of a workflow file issue."

Root cause: on.push.branches / on.pull_request.branches were set to ${{ vars.PRIMARY_BRANCH || 'main' }} / ${{ vars.DEVELOP_BRANCH || 'develop' }}. GitHub Actions does not support context expressions in on: trigger filters — they're evaluated by the platform before a workflow run is even created. A workflow file using an expression there is rejected outright, regardless of what the rest of the file contains. env:/step-level ${{ vars.X }} usage elsewhere in these same files is fine — only the on: block is affected.

This means ci.yml — the repo's main "CI Pipeline" workflow, with all 7-8 phases including ci-gate — has never actually executed in this repository, on any push or pull_request, since this line was written. All the mypy/pre-commit/ci-gate work below existed only on paper until this fix. Fixed by hardcoding the branch names the || 'default' fallback already implied (main, develop where applicable) across all 5 affected workflows.

1. Pre-commit hooks are now enforced in CI, not just locally

.pre-commit-config.yaml has 15+ hooks (ruff, mypy, contract scanner, deprecated-API checks, gitleaks, ...), but nothing ran them except a contributor's local git commit — and only if they'd run pre-commit install. There was no CI-side backstop.

Added a precommit job to .github/workflows/ci.yml (Phase 2.5) that runs pre-commit run --all-files via pre-commit/action on every PR/push, and made it required by ci-gate. Two hooks are skipped with a documented reason (SKIP=gitleaks,packet-envelope-prohibited):

  • gitleaks's pre-commit hook runs gitleaks protect --staged, which inspects the git index — that has no equivalent in a plain CI checkout. Secret scanning is already handled correctly elsewhere in ci.yml via gitleaks/gitleaks-action.
  • packet-envelope-prohibited currently has 23 pre-existing violations across engine//chassis/ — a real, tracked architecture migration (PacketEnvelopeTransportPacket from constellation_node_sdk, see docs/contracts/SHARED_MODELS.md), not a CI-config problem. Skipping it here keeps this job actionable now instead of permanently red on unrelated, out-of-scope migration debt. (It turns out .github/workflows/terminology-guard.yml also runs this same check natively, independent of pre-commit — see "additional findings" below.)

Running the full hook suite locally also surfaced and let me fix three small, low-risk pre-existing issues that were blocking hooks from ever passing:

  • l9-contract-scan had 3 CRITICAL false-positive findings (Cypher label interpolation) in engine/causal/attribution.py/causal_compiler.py — the labels are sanitized via sanitize_label(), just a few lines above the interpolation, which the single-line regex can't see. Two other files already had this exact allowlist entry for this exact reason; added these two.
  • .hypothesis/ (gitignored) had 92 cache files already tracked from before the ignore rule existed, causing constant unrelated whitespace-hook noise. Untracked them (this is why the file-count diff below looks big — the actual code diff is small).
  • 5 markdown docs had trailing whitespace / missing EOF newlines.

2. Fixed mypy's CI step, which has effectively never run

ci.yml's "Run Mypy Type Checker" step ran mypy . (whole repo) piped through || echo "... non-blocking". Turns out mypy . has always failed immediately with Source file found twice under different module names (tools/auditors/base.py vs auditors.base) before checking a single file — so this step was a silent no-op even setting aside issue #0 above. Fixed the scope to match the working, already-verified invocation (mypy engine/ --config-file=pyproject.toml --ignore-missing-imports --exclude chassis, same as make lint / ci-quality.yml) and removed the silencing, so real type errors now fail the job.

3. Fixed quality-gate in ci-quality.yml, which never actually checked most of its inputs

needs.lint-format.result, needs.secrets-scan.result, and needs.yaml-validate.result used GitHub Actions dot notation on hyphenated job IDs- is parsed as subtraction in that syntax, so all three silently evaluated to an empty string instead of the job's real result. Every if [[ "" == "failure" ]] was therefore always false: this gate could report ✅ even when lint-format or secrets-scan failed. semgrep's result also wasn't checked at all. Fixed with bracket syntax (needs['lint-format'].result) and added the missing semgrep check.

4. Fail-open testing: surface every failure in one run, still block on any of them

pytest.ini had -x in addopts, applied globally (including the pytest-unit pre-commit hook and every CI test-suite run). -x stops at the very first failing test — so a run with N failures only ever reports failure #1, and fixing it just reveals #2 on the next run. Removed -x; added -ra so a short summary of every non-passing test prints at the end. Still exits non-zero (still fails the job) on any failure — this only changes how much a single run surfaces, not whether it blocks.

Verification

  • ruff check . / ruff format --check . / mypy engine/ ... all exit 0
  • pre-commit run --all-files SKIP=gitleaks,packet-envelope-prohibited exits 0 locally (every hook either passes or is a documented skip)
  • Removing -x verified to surface multiple failures in one run instead of stopping at the first (tested against a file with 4 pre-existing failures)
  • All workflow YAML re-validated with yaml.safe_load, plus a targeted scan confirming no remaining vars. expressions inside any on: block
  • The on: trigger fix itself can't be verified via a live run on this PR, since ci.yml's trigger is (correctly) scoped to main/develop, and this PR's base is a feature branch (see below) — but the root cause and fix are unambiguous per GitHub's documented behavior, and match the exact 0-second/0-job failure signature observed via gh run list both before and after other unrelated commits, confirming it's a trigger-level, not content-level, problem

Additional pre-existing CI issues discovered (NOT fixed here — out of narrow scope, flagging for visibility)

Confirmed all of the following are pre-existing and unrelated to any change in this PR or #125:

  • contracts.yml runs a third, separate mypy engine/ --strict ... invocation (ci.yml and ci-quality.yml are the other two, both now fixed/consistent) that surfaces 16 --strict-only errors (missing type annotations/generic params) in engine/inference_rule_registry.py, engine/graph/graph_sync_client_fix.py, engine/startup_wiring.py, engine/graph/community_export.py, plus one stale type: ignore in multihop.py this stack made newly-unused. Three inconsistent mypy invocations across three workflows is itself a consistency problem, but fixing the underlying --strict gaps is a distinct, larger task.
  • terminology-guard.yml independently runs check_packet_envelope_prohibited.py (same 23 violations noted above) as its own CI step, unrelated to the precommit job's SKIP.
  • docs-code-sync.yml's "Check Markdown Links" step has a bash bug: target="${link./}" is invalid parameter-substitution syntax (bad substitution), so it fails on any PR touching markdown docs, regardless of link validity.
  • pr-review-enforcement.yml's size-policy bot flags this PR for file count (102 files) — almost entirely the .hypothesis/ untracking (92 deletions); the actual code diff is ~10 files.
  • Crucially: get_ci_status reports Required checks: 0 failed, 0 pending on both this PR and Enable working CI lint autofix + remediate pre-existing ruff/mypy/pytest errors #125 — i.e. none of these checks (including the now-fixed ci-gate/quality-gate) are currently configured as required status checks in branch protection, so nothing here actually blocks a merge today regardless of red/green. This is likely the single biggest remaining gap relative to "PRs with CI failures should be blocked" — see below.

Not fixed here (flagged, deliberately out of scope)

  • packet-envelope-prohibited's 23 violations — real architecture migration, needs its own dedicated effort.
  • Branch protection / required status checks: this run's gh access doesn't have permission to view or modify branch protection rules (403 on GET .../branches/main/protection). For ci-gate and quality-gate to actually block merges (not just show red), a repo admin needs to mark them (and ideally precommit) as required status checks in Settings → Branches. Noted in docs/CI_PIPELINE.md.
  • Several other pre-existing test-content bugs (e.g. tests/security/test_compliance_security.py has the same DomainPackLoader(domains_dir=...) bug fixed in Enable working CI lint autofix + remediate pre-existing ruff/mypy/pytest errors #125's test_injection.py) are left alone — those are codebase content, not CI plumbing, and out of scope for "narrow to the CI pipeline itself."

Base branch note

This PR is based on cursor/enable-ci-autofix-ddaf (#125) rather than main, since the new precommit/mypy CI jobs need that branch's lint/mypy/pytest-config fixes to actually pass. Merge order: #125 → this PR → main. Because of this, ci.yml's pull_request trigger (scoped to main/develop bases) won't fire on this PR itself — the fix from item #0 will start taking effect once this lands on main/develop.

Open in Web Open in Cursor 

cursoragent and others added 7 commits July 19, 2026 21:57
.hypothesis/ has been in .gitignore all along, but 92 of its cache
files were already committed to the repo at some point before the
ignore rule was added (git only ignores new paths, it doesn't untrack
existing ones). Every test run that exercises Hypothesis-based property
tests mutates these files, so they showed up as noisy, unrelated diffs
every time `pre-commit run --all-files` (trailing-whitespace,
end-of-file-fixer) touched them locally or in CI. Untracking them so
the ignore rule actually takes effect.

Co-authored-by: Igor Beylin <cryptoxdog@users.noreply.github.com>
trailing-whitespace / end-of-file-fixer (from .pre-commit-config.yaml)
were failing on 5 pre-existing docs. Whitespace-only changes, no
content edits.

Co-authored-by: Igor Beylin <cryptoxdog@users.noreply.github.com>
tools/contract_scanner.py's SEC-001 rule (Cypher label interpolation
without sanitize_label()) is a single-line regex; it already allowlists
engine/sync/generator.py and engine/handlers.py with the comment
"labels sanitized via sanitize_label()" for exactly this pattern: the
label variable is sanitized a few lines above the f-string that embeds
it, so the same-line regex can't see it.

engine/causal/attribution.py:72 and engine/causal/causal_compiler.py:
62-63 hit the identical pattern (sanitize_label() called on the
preceding lines: attribution.py:60,63 and causal_compiler.py:50-51) but
were never added to the allowlist, so `l9-contract-scan` currently
reports 3 false-positive CRITICAL violations and blocks every commit
that touches those files (or, since pre-commit was never wired into
CI, blocks nothing and nobody -- see the CI-pipeline PR). Verified both
files pass `python tools/contract_scanner.py <file>` clean after this
change.

Co-authored-by: Igor Beylin <cryptoxdog@users.noreply.github.com>
…ust one

pytest.ini's addopts had -x, applied globally to every pytest invocation
that doesn't override addopts -- including the pytest-unit pre-commit
hook and ci.yml's/ci-quality.yml's full test-suite runs. -x aborts the
entire run after the very first failing test, so a run with N failing
tests only ever reports test #1: fixing it, re-running, and discovering
test #2 takes N push/CI round-trips instead of one.

Removed -x; added -ra so the run also prints a one-line summary of every
non-passing test (failed, error, or skipped-with-reason) at the end.
Runs still exit non-zero (fail the job) if anything fails -- this only
changes how much is surfaced in a single run, not whether failures
block.

Verified: running a file with 4 pre-existing failures now reports all 4
in one pass instead of stopping after the first (previously reproduced
with -x).

Co-authored-by: Igor Beylin <cryptoxdog@users.noreply.github.com>
Two changes to .github/workflows/ci.yml:

1. New "precommit" job (Phase 2.5), required by ci-gate. Runs
   `pre-commit run --all-files` via pre-commit/action so every hook in
   .pre-commit-config.yaml is enforced on every PR/push -- regardless of
   whether a contributor (human or agent) ran `pre-commit install`
   locally, or bypassed hooks with `git commit -n`. Skips two hooks with
   SKIP=gitleaks,packet-envelope-prohibited (both explained inline and
   in docs/CI_PIPELINE.md): gitleaks's `--staged` semantics don't apply
   to a plain CI checkout and secret scanning is already handled
   correctly by the `security` job's gitleaks-action; the packet-envelope
   hook has 23 pre-existing violations representing a real, separately
   tracked architecture migration (PacketEnvelope -> TransportPacket),
   not a CI-config problem.

2. Fixed the "Run Mypy Type Checker" step in the lint job. It ran
   `mypy .` (whole repo) piped through `|| echo "... non-blocking"`,
   which silenced every result -- and `mypy .` itself has always failed
   immediately with "Source file found twice under different module
   names" (tools/auditors/base.py vs auditors.base) before checking a
   single file, so this step has effectively always been a no-op. Now
   runs the same scoped, verified-working invocation as `make lint` /
   ci-quality.yml (`mypy engine/ --config-file=pyproject.toml
   --ignore-missing-imports --exclude chassis`), with no silencing, so
   real type errors now fail the job.

ci-gate's fan-in now also requires `precommit` and fails if it fails.

Verified locally: `pre-commit run --all-files
SKIP=gitleaks,packet-envelope-prohibited` exits 0 on the current tree
(after the preceding commits in this PR); `mypy engine/ ...` exits 0.

Co-authored-by: Igor Beylin <cryptoxdog@users.noreply.github.com>
ci-quality.yml's quality-gate job referenced needs.lint-format.result,
needs.secrets-scan.result, and needs.yaml-validate.result using GitHub
Actions dot notation. Dot notation doesn't work for job IDs containing a
hyphen -- `-` is parsed as subtraction -- so those three always
evaluated to an empty string, not the job's actual result. Every `if
[[ "" == "failure" ]]` was therefore always false: quality-gate could
report a passing summary even when lint-format or secrets-scan failed.
semgrep's result wasn't checked at all (missing from the fail
conditions, independent of the hyphen bug).

Fixed by switching to bracket syntax (needs['lint-format'].result, etc.)
for the three hyphenated job IDs, and added the missing semgrep check.

Co-authored-by: Igor Beylin <cryptoxdog@users.noreply.github.com>
…ate fix

Documents the new Phase 2.5 (precommit job), the mypy scope/blocking
fix, the ci-quality.yml quality-gate hyphen-notation bug fix, the
pytest -x removal, and the current SKIP list for the precommit job with
rationale for each skipped hook. Also notes that ci-gate and
quality-gate need to be configured as required status checks in branch
protection to actually block merges -- a workflow job failing does not,
by itself, prevent merging.

Co-authored-by: Igor Beylin <cryptoxdog@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown

Too Many Files Changed
Changed: 102 files
Limit: 50 files
Action Required: Split into multiple focused PRs

⚠️ Large PR Warning
Lines changed: 659
Warning threshold: 300 lines
Consider splitting for easier review

📋 Best Practices for Large Changes

  1. Refactoring + Features: Separate into 2 PRs
  2. Multiple Features: One PR per feature
  3. Database + Code: Separate migration from logic
  4. Generated Code: Separate from manual changes

🚫 This PR is blocked from merging until size limits are met.

cursoragent and others added 2 commits July 19, 2026 22:07
…rkflows never ran

ci.yml, docker-build.yml, supply-chain.yml, codeql.yml, and
release-drafter.yml all had `on.push.branches` / `on.pull_request.branches`
set to `${{ vars.PRIMARY_BRANCH || 'main' }}` / `${{ vars.DEVELOP_BRANCH
|| 'develop' }}`.

GitHub Actions does not support context expressions in `on:` trigger
filters -- they're evaluated by the platform before a workflow run is
even created, so `vars`/`env`/etc. aren't available there (they are
supported in `env:`, job steps, etc. -- just not `on:`). A workflow file
using an expression there is rejected outright: "This run likely failed
because of a workflow file issue", 0 jobs created, regardless of what
the workflow actually contains.

Confirmed via `gh run list`: every push-triggered run of these 5 files
shows `failure` at `0s` duration, and none of them ever appear as a
successful pull_request-triggered run either -- meaning ci.yml (the
main "CI Pipeline" workflow, including everything fixed earlier in this
PR) has never actually executed in this repository, on any event, since
this line was written.

Fixed by hardcoding the branch names the `|| 'default'` fallback already
implied (main, develop where applicable). This is the single highest-
leverage fix in this PR: none of the mypy/pre-commit/quality-gate fixes
elsewhere in this stack could ever be verified by real CI without it.

Co-authored-by: Igor Beylin <cryptoxdog@users.noreply.github.com>
Co-authored-by: Igor Beylin <cryptoxdog@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown

Too Many Files Changed
Changed: 106 files
Limit: 50 files
Action Required: Split into multiple focused PRs

⚠️ Large PR Warning
Lines changed: 735
Warning threshold: 300 lines
Consider splitting for easier review

📋 Best Practices for Large Changes

  1. Refactoring + Features: Separate into 2 PRs
  2. Multiple Features: One PR per feature
  3. Database + Code: Separate migration from logic
  4. Generated Code: Separate from manual changes

🚫 This PR is blocked from merging until size limits are met.

@cursor cursor Bot changed the title Wire pre-commit into CI + fix CI gates that never actually blocked Wire pre-commit into CI, fix triggers that never ran, fix gates that never blocked Jul 19, 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.

2 participants