Skip to content

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

Closed
cryptoxdog wants to merge 10 commits into
fix/workflow-triggers-gatesfrom
cursor/repair-ci-pipeline-precommit-ddaf
Closed

Wire pre-commit into CI, fix triggers that never ran, fix gates that never blocked#126
cryptoxdog wants to merge 10 commits into
fix/workflow-triggers-gatesfrom
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 

@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.

@github-actions

github-actions Bot commented Jul 19, 2026

Copy link
Copy Markdown

ℹ️ Ignored 92 generated/cache file(s) from size accounting.

⚠️ Large PR Warning
Reviewable lines changed: 635
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: Exclude it from reviewable-size accounting

This PR passes the blocking limit but is larger than recommended.

@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
@cryptoxdog
cryptoxdog force-pushed the cursor/repair-ci-pipeline-precommit-ddaf branch from c784f26 to d554094 Compare July 22, 2026 02:11
cryptoxdog added a commit that referenced this pull request Jul 22, 2026
- Add hypothesis>=6.100,<7 to requirements-ci.txt: tests/property/ requires
  hypothesis, declared only in the Poetry dev group; CI installs via
  requirements files, so Contract Enforcement test collection fails on main.
- Remove dead link docs/DEPLOYMENT.md -> ../iac/README.md (no iac/ directory
  exists), which fails Docs-Code Sync 'Check Markdown Links' on main.

Both defects pre-date the PR #125/#126 CI-foundation remediation and were
exposed once the gates began running. Verified locally: tests/property/
passes (11 tests), docs relative-link check clean.

Co-authored-by: Manus PR Remediator <pr-remediator@manus.bot>
manus-ci-agent and others added 10 commits July 22, 2026 20:23
Filtered extraction from #125 (commit 9b0d03c), workflows only:

- ci.yml / codeql.yml / docker-build.yml / supply-chain.yml /
  release-drafter.yml: replace ${{ vars.* }} branch triggers with
  literal main/develop — GitHub never evaluates expressions in the
  'on:' block, so these workflows never triggered
- ci.yml: install lint deps from requirements-ci.txt (canonical set)
  and make mypy consistent + blocking (was || echo non-blocking)
- compliance.yml: delegate Cypher injection check to the canonical
  tools/contract_scanner.py instead of a second divergent regex;
  align mypy invocation with the CI gate (drop --strict divergence)
- refactoring-validation.yml: gate the deep validation to refactor/
  branches or manual dispatch; align mypy invocation
- ci-quality.yml: fix needs.lint-format / needs.secrets-scan /
  needs.yaml-validate expressions to bracket syntax (hyphenated job
  ids resolve to empty string with dot syntax, so gates never fired);
  add missing semgrep failure branch to the quality-gate summary

Deliberately excluded per review: replacement Semgrep runner,
GitGuardian fail-open gating, coverage -m unit narrowing and the
five --ignore test exclusions, tools/contract_scanner.py hunk (#129).

All 8 workflow YAMLs parse cleanly.
.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>
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>
…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>
…de-sync, pr-review-enforcement, terminology-guard

Applied from cognitive-engine-graphs-open-pr-remediation-v2 126/apply.sh on the rebased #125 baseline.
Post-checks passed: git apply --check, git diff --check, YAML validation on all four workflows, changed-path allowlist.
@cryptoxdog
cryptoxdog force-pushed the cursor/repair-ci-pipeline-precommit-ddaf branch from 125ec99 to 20aeb3d Compare July 22, 2026 20:32
@cryptoxdog
cryptoxdog changed the base branch from cursor/enable-ci-autofix-ddaf to fix/workflow-triggers-gates July 22, 2026 20:32
@sonarqubecloud

Copy link
Copy Markdown

Copy link
Copy Markdown
Collaborator Author

Restacked. This PR's 9 commits have been rebased onto fix/workflow-triggers-gates (#135) and the base retargeted accordingly, because its former base (#125's branch) is being replaced by the split series #129#135.

Why stacked on #135 instead of main: this branch's trigger/quality-gate fixes were inherited from #125's branch; rebasing directly onto main silently reverted the literal on: triggers back to the broken ${{ vars.* }} expressions. Stacking on #135 preserves them.

Contents after restack (vs #135): 92 .hypothesis/ cache untrackings, pre-commit wired into CI (ci.yml precommit job + ci-gate), quality-gate hyphenated-job fix retained, four-workflow remediation (contracts, docs-code-sync, pr-review-enforcement, terminology-guard), compliance/contracts mypy alignment, and docs updates. All 10 touched workflow YAMLs parse.

Action for maintainer: once #135 merges, GitHub will auto-retarget this PR to main. Mark ready for review after checks pass.

Copy link
Copy Markdown
Collaborator Author

Closing as decomposed, per the remediation plan. Its content is now fully accounted for: the vars-expression trigger fixes are in #135; everything else clean was extracted into #137 (stack 9/9 on top of #136) — .hypothesis untracking, docs link checker repair, PR-size generated-file handling, quality-gate fix documentation, CI pipeline docs, terminology changed-file enforcement, and pre-commit CI wiring. The raw SKIP=packet-envelope-prohibited model was deliberately NOT preserved: #137 skips only gitleaks, leaving the precommit job honestly red on the 23 PacketEnvelope references until the CI baseline-ratchet system (R1–R6) lands with a governed per-finding debt ledger.

@cryptoxdog cryptoxdog closed this Jul 22, 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