From 2b4345c1809562ab9075163807503340574c7015 Mon Sep 17 00:00:00 2001 From: haeter525 Date: Sat, 20 Jun 2026 04:37:11 +0000 Subject: [PATCH 1/5] feat(skills): add dependency-update PR validator skill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dependabot PRs on ev-flow/quark-engine look green but often aren't — CI hardcodes some package versions (pytest.yml:53) independent of setup.py, and unrelated flakes/baseline drift get blamed on the bump. This skill checks actual installed version vs target, hardcoded workflow pins, and pip resolvability of the post-PR setup.py before drafting a merge/escalate recommendation. Never posts to GitHub itself. Validated against 5 real PRs (923, 922, 921, 906, 893) plus 6 live eval runs comparing with-skill vs baseline. Refs 18z/QuarkHQ#3 Co-Authored-By: Claude Sonnet 4.6 --- .../dependency-update-pr-validator/SKILL.md | 224 ++++++++++++++++++ .../evals/evals.json | 26 ++ .../examples/escalation_comment.md | 107 +++++++++ .../examples/ready_to_merge.md | 28 +++ .../scripts/check_actual_version.sh | 46 ++++ .../scripts/check_ci.sh | 28 +++ .../scripts/check_pip_resolution.sh | 59 +++++ .../scripts/check_workflow_pin.sh | 26 ++ .../scripts/compare_versions.sh | 18 ++ .../scripts/parse_pr.sh | 42 ++++ .../scripts/run_checkpoint.sh | 18 ++ 11 files changed, 622 insertions(+) create mode 100644 .claude/skills/dependency-update-pr-validator/SKILL.md create mode 100644 .claude/skills/dependency-update-pr-validator/evals/evals.json create mode 100644 .claude/skills/dependency-update-pr-validator/examples/escalation_comment.md create mode 100644 .claude/skills/dependency-update-pr-validator/examples/ready_to_merge.md create mode 100755 .claude/skills/dependency-update-pr-validator/scripts/check_actual_version.sh create mode 100755 .claude/skills/dependency-update-pr-validator/scripts/check_ci.sh create mode 100755 .claude/skills/dependency-update-pr-validator/scripts/check_pip_resolution.sh create mode 100755 .claude/skills/dependency-update-pr-validator/scripts/check_workflow_pin.sh create mode 100755 .claude/skills/dependency-update-pr-validator/scripts/compare_versions.sh create mode 100755 .claude/skills/dependency-update-pr-validator/scripts/parse_pr.sh create mode 100755 .claude/skills/dependency-update-pr-validator/scripts/run_checkpoint.sh diff --git a/.claude/skills/dependency-update-pr-validator/SKILL.md b/.claude/skills/dependency-update-pr-validator/SKILL.md new file mode 100644 index 000000000..64eb1503f --- /dev/null +++ b/.claude/skills/dependency-update-pr-validator/SKILL.md @@ -0,0 +1,224 @@ +--- +name: dependency-update-pr-validator +description: Validates dependabot dependency-update PRs against quark-engine's CI and drafts a merge/escalate recommendation. Use when asked to review, validate, or triage a dependabot PR on ev-flow/quark-engine, or when the user references issue 18z/QuarkHQ#3. +version: 1.0.0 +allowed-tools: bash(gh:*), bash(.claude/skills/dependency-update-pr-validator/scripts/*.sh) +--- + +# Dependency Update PR Validator + +## Why this exists + +Dependabot opens a steady stream of near-identical, high-volume PRs against +`ev-flow/quark-engine` (one per dependency bump). Reviewing each one by hand +— does CI actually pass, does it actually test the new version — is +repetitive and easy to rubber-stamp. This skill automates that check and +produces a recommendation for a human to act on. See +[18z/QuarkHQ#3](https://github.com/18z/QuarkHQ/issues/3) for the original +ask. + +**This skill never writes to GitHub.** It never calls `gh pr comment`, +`gh pr merge`, or `gh pr review`. Every output is a draft for a human to +read, edit, and post themselves — these PRs live in the upstream repo +(`ev-flow/quark-engine`), not the user's fork, so posting or merging there +is a visible, shared-state action that needs a human in the loop every +time. + +## The core insight: green CI does not mean the target version was tested + +This repo declares dependencies in two independent places that drift apart: + +- `setup.py` — actually consumed by `pip install .` in `pytest.yml`. +- `Pipfile` / `Pipfile.lock` — **not consumed by any current workflow.** + No workflow runs `pipenv install`. + +On top of that, `pytest.yml` has a standalone line that hardcodes some +versions outright, e.g. (as of this writing): + +``` +python -m pip install langchain==0.2.11 langchain-core==0.2.23 langchain-openai==0.1.17 --upgrade +``` + +That line ignores `setup.py` entirely. A dependabot PR that bumps +`langchain-core` in `setup.py` will still show all-green CI — CI just +silently kept testing the old pinned version. Conversely, fully unpinned +transitive deps (e.g. `idna`, `requests`) often float to whatever's latest +at install time regardless of which file dependabot touched, and that +floating version is frequently already at-or-above the dependabot target — +which is a legitimate pass, not a coincidence to be suspicious of. + +The lesson: **don't infer anything from which file the PR changed.** +Always directly check (a) what version actually got installed in the CI +run, and (b) whether some workflow step hardcodes this exact package to an +older version. Those two checks are simple greps; trust them over the +filename. + +## Workflow + +Run each step and **print its result before moving to the next one** — +the slow part is digging through CI logs, so show the cheap facts +(package, before/target version, CI pass/fail) first rather than holding +everything for one final report. + +### Step 1 — Parse the PR (cheap, do this first) + +```bash +bash .claude/skills/dependency-update-pr-validator/scripts/run_checkpoint.sh +``` + +This prints the PR title, package name, before version, target version, +and whether every CI check passed (treating `skipping` as fine — e.g. +`kali-package.yml` only runs on push to master, so it's always skipped on +a PR; that is not a failure). + +Show this checkpoint to the user immediately. + +### Step 2 — If `setup.py` was changed: confirm the new pin is actually installable + +Run this whenever `setup.py` is among the PR's changed files (check +`changed_files` from Step 1) — **regardless of CI pass/fail**. This is a +static check of the PR's own diff, not of what CI happened to test, so a +CI failure in Step 3 doesn't make it irrelevant — a PR can fail CI for an +unrelated reason *and* be independently broken here, and you want to catch +both. + +```bash +bash .claude/skills/dependency-update-pr-validator/scripts/check_pip_resolution.sh +``` + +A dependabot PR usually bumps one pin inside a requirements list (e.g. +`quarkAgentRequirements` in `setup.py`) while leaving its sibling pins in +the same list untouched. That combination can be mutually unsatisfiable — +`pip install` for those exact pinned versions together raises +`ResolutionImpossible` — even when CI is fully green, because CI's +hardcoded install step (Step 4) tests its own separate versions and never +actually tries to install what `setup.py` now declares. This is a +stronger, more definitive reason to block a merge than `is_covered`: if +real users can't even `pip install` the result, it doesn't matter what CI +tested. If this check reports `pip_resolution=conflict`, the recommendation +is **always** "Needs human review," regardless of what CI/is_covered says. + +### Step 3 — If any check failed: find the real root cause, don't just report "CI failed" + +A red check on a dependabot PR is frequently **not caused by the bump at +all**. Two patterns observed in practice (see +[examples/escalation_comment.md](examples/escalation_comment.md) for full +writeups): + +- **Environment flake** — a test fixture fails to download a valid file + (e.g. `zipfile.BadZipFile: File is not a zip file` while fetching a + sample APK). Unrelated to any dependency. +- **Stale-branch baseline drift** — `smoke_test.yml`'s hardcoded expected + behavior-counts (e.g. "Ahmyth.apk should show 39 behaviors") no longer + match because master gained new detection rules after the dependabot + branch was cut. Also unrelated to the dependency bump. + +To find the actual cause: + +```bash +gh run view --repo --log-failed +``` + +Read the failure, classify it as "looks like a real regression from this +bump" vs. "looks unrelated" (give your reasoning — e.g. the failing +package is a test-only tool that can't plausibly affect malware-detection +counts), and stop here (skip Step 4 — there's no successful build log to +read a version out of). This is always an escalation regardless of +classification — only difference is what you tell the human, and Step 2's +finding (if any) still applies on top of it. + +### Step 4 — If CI passed: check what was actually installed + +```bash +bash .claude/skills/dependency-update-pr-validator/scripts/check_actual_version.sh +``` + +This finds the `pytest.yml` ("build") run for the PR's branch and greps +its log for the package's `Successfully installed` line — the one place +that reflects what pip genuinely resolved and tested, regardless of which +declaration file the PR touched. + +Then check whether a workflow hardcodes this package to a stale version +(this is what explains a "No" result, when there is one): + +```bash +bash .claude/skills/dependency-update-pr-validator/scripts/check_workflow_pin.sh +``` + +Then compute is_covered: + +```bash +bash .claude/skills/dependency-update-pr-validator/scripts/compare_versions.sh +``` + +### Step 5 — Print the result table, then ask before posting anything + +Always finish with **one markdown table in English**, in this exact field +order, regardless of which branch you took: + +``` +| Field | Value | +|---|---| +| Package | | +| Before version | | +| Target version | | +| CI checks | <✅ All passed / ❌ N failing, with a short reason> | +| Actual installed version | | +| Is covered (actual ≥ target) | <✅ Yes / ❌ No / — Not verified> | +``` + +Right after the table, add whatever prose is needed to explain *why* — +quote the hardcoded pin line if `check_workflow_pin.sh` found one, quote +the `pip install` conflict if `check_pip_resolution.sh` reported one, or +give the root-cause classification from Step 3 if CI failed — then one +line: **Recommendation: Ready to merge.** or **Recommendation: Needs human +review.** A `pip_resolution=conflict` result always forces "Needs human +review," even if CI passed and is_covered is Yes. See +[examples/ready_to_merge.md](examples/ready_to_merge.md) and +[examples/escalation_comment.md](examples/escalation_comment.md) for full +worked examples in this exact format. + +Then **ask the user**, every time, whether to post this as a comment on +the PR — e.g. "Would you like me to post this as a comment on PR #923?". +Only call `gh pr comment --repo --body "..."` if they say yes. +Never post automatically, and never call `gh pr merge` regardless of the +recommendation — merging is the human's call to make after reading this. + +Present the draft to the user. Do not post it anywhere. + +## Scripts reference + +| Script | Purpose | +|---|---| +| `scripts/run_checkpoint.sh ` | Step 1: package, before/target version, CI pass/fail | +| `scripts/parse_pr.sh ` | Just the dependabot body/diff parsing part of step 1 | +| `scripts/check_ci.sh ` | Just the CI-checks part of step 1 | +| `scripts/check_pip_resolution.sh ` | Step 2 (only if `setup.py` changed, regardless of CI outcome): is the bumped pin pip-installable alongside its sibling pins in the same requirements list? | +| `scripts/check_actual_version.sh ` | Step 4: actual installed version from the pytest.yml build log | +| `scripts/check_workflow_pin.sh ` | Step 4: does a workflow hardcode this package to an older version? | +| `scripts/compare_versions.sh ` | Step 4: is_covered (PEP 440 version comparison) | + +All scripts default to the real PR repo when you pass `ev-flow/quark-engine` +as `` — that's where every dependabot PR for this project lives, not +the user's fork. + +## Validated against + +This logic was walked through manually against five real dependabot PRs +before being written down here — keep these as a sanity check if you +change the logic: + +| PR | Package | Outcome | +|---|---|---| +| [#923](https://github.com/ev-flow/quark-engine/pull/923) | idna | Ready to merge — CI green, actual version matches target, no pin found | +| [#922](https://github.com/ev-flow/quark-engine/pull/922) | langchain-core | Needs human — CI green but `pytest.yml:53` pins an older version; target never installed. Also: `pip install` for the post-PR `setup.py` (`langchain==0.2.11 langchain-core==1.3.3 langchain-openai==0.1.17`) raises `ResolutionImpossible` — broken regardless of CI | +| [#921](https://github.com/ev-flow/quark-engine/pull/921) | langchain | Needs human — CI red, root cause is an unrelated sample-download flake. Also independently fails `check_pip_resolution.sh`: `langchain==0.3.30` conflicts with the untouched sibling pin `langchain-core==0.2.23` — broken even after a clean re-run | +| [#906](https://github.com/ev-flow/quark-engine/pull/906) | pytest (Pipfile.lock) | Needs human — CI red, but root cause is smoke-test baseline drift from a stale branch, unrelated to pytest | +| [#893](https://github.com/ev-flow/quark-engine/pull/893) | requests (Pipfile.lock) | Ready to merge — CI green, actual version (resolved transitively) exceeds target, no pin found | + +The `check_pip_resolution.sh` step (and the `ResolutionImpossible` finding +for #922 above) was added after an eval comparison against a baseline +agent without this skill — it independently ran `pip install` on the +post-PR `setup.py` and caught a more definitive, CI-independent reason to +block the PR. See `evals/` in this skill directory for the eval set this +was validated against. diff --git a/.claude/skills/dependency-update-pr-validator/evals/evals.json b/.claude/skills/dependency-update-pr-validator/evals/evals.json new file mode 100644 index 000000000..ca6ba2797 --- /dev/null +++ b/.claude/skills/dependency-update-pr-validator/evals/evals.json @@ -0,0 +1,26 @@ +{ + "skill_name": "dependency-update-pr-validator", + "evals": [ + { + "id": 0, + "eval_name": "clean-ready-to-merge", + "prompt": "Please use the dependency-update-pr-validator skill to check ev-flow/quark-engine PR #923 (https://github.com/ev-flow/quark-engine/pull/923) and tell me whether it's ready to merge.", + "expected_output": "Package idna, before 3.11, target 3.15, CI all passed, actual installed version 3.15, is_covered Yes, recommendation Ready to merge, ends by asking whether to post a PR comment.", + "files": [] + }, + { + "id": 1, + "eval_name": "structural-version-pin", + "prompt": "Can you validate this dependabot PR for quark-engine? https://github.com/ev-flow/quark-engine/pull/922 — is it safe to merge?", + "expected_output": "Package langchain-core, before 0.2.23, target 1.3.3, CI all passed, actual installed version still 0.2.23, is_covered No, explanation references the hardcoded pin in pytest.yml (line ~53), recommendation Needs human review, ends by asking whether to post a PR comment.", + "files": [] + }, + { + "id": 2, + "eval_name": "unrelated-ci-failure", + "prompt": "There's a dependabot PR for quark-engine that's failing CI, can you look into why? https://github.com/ev-flow/quark-engine/pull/921", + "expected_output": "Package langchain, before 0.2.11, target 0.3.30, CI failing, root cause identified as an unrelated sample-download/zip failure (not the langchain bump), is_covered not verified, recommendation Needs human review (suggest re-run/rebase), ends by asking whether to post a PR comment.", + "files": [] + } + ] +} diff --git a/.claude/skills/dependency-update-pr-validator/examples/escalation_comment.md b/.claude/skills/dependency-update-pr-validator/examples/escalation_comment.md new file mode 100644 index 000000000..61201aaaf --- /dev/null +++ b/.claude/skills/dependency-update-pr-validator/examples/escalation_comment.md @@ -0,0 +1,107 @@ +# Example: Escalation comment drafts + +## Case A — structural version pin (PR #922, langchain-core) + +CI was green, but the green result is misleading. + +``` +| Field | Value | +|---|---| +| Package | langchain-core | +| Before version | 0.2.23 | +| Target version | 1.3.3 | +| CI checks | ✅ All passed (push is skipping, not a failure) | +| Actual installed version | 0.2.23 (unchanged!) | +| Is covered (actual ≥ target) | ❌ No | +``` + +`pytest.yml` line 53 hardcodes +`pip install langchain==0.2.11 langchain-core==0.2.23 langchain-openai==0.1.17`, +independent of what `setup.py` declares. This PR only bumped `setup.py`, +so CI kept installing and testing the old version 0.2.23. The green CI +run does not validate langchain-core 1.3.3 at all. + +On top of that, `check_pip_resolution.sh` shows the bump is broken on its +own terms, independent of CI: `setup.py`'s `quarkAgentRequirements` list +now reads `langchain==0.2.11 langchain-core==1.3.3 langchain-openai==0.1.17`, +and `pip install` for that exact combination fails — +`langchain==0.2.11` requires `langchain-core<0.3.0,>=0.2.23`, which +`1.3.3` violates: + +``` +ERROR: Cannot install langchain-core==1.3.3 and langchain==0.2.11 because +these package versions have conflicting dependencies. +ERROR: ResolutionImpossible +``` + +Anyone who runs `pip install -e ".[QuarkAgent]"` after this merges gets a +hard failure. This isn't a CI gap, it's the PR's own diff being internally +inconsistent — `langchain` and `langchain-openai` need to move together +with `langchain-core`, not as an isolated single-pin bump. + +**Recommendation: Needs human review.** + +Would you like me to post this as a comment on PR #922? + +## Case B — CI failure unrelated to the bump (PR #921, langchain) + +``` +| Field | Value | +|---|---| +| Package | langchain | +| Before version | 0.2.11 | +| Target version | 0.3.30 | +| CI checks | ❌ 1 failing (`build`, pytest.yml) | +| Actual installed version | Not verified (CI failed before this check) | +| Is covered (actual ≥ target) | — Not verified | +``` + +All 461 test errors trace back to one fixture failing to download a valid +zip/APK file in `tests/core/test_apkinfo.py` +(`zipfile.BadZipFile: File is not a zip file` loading +`13667fe3b0ad496a0cd157f34b7e0c991d72a4db.apk`), not to anything in +`langchain`. This looks like a one-off sample-download flake rather than a +regression from the version bump. + +This PR also changes `setup.py`, so `check_pip_resolution.sh` ran +independently of the CI result above (it's a static check, not tied to +whether CI happened to pass) — and it also found a conflict: +`langchain==0.3.30` together with the untouched sibling pin +`langchain-core==0.2.23` is `ResolutionImpossible`. So even after the CI +flake is resolved with a clean re-run, this PR would still need +`langchain-core` bumped alongside `langchain` before it's mergeable. + +**Recommendation: Needs human review** — suggest re-running CI (or +rebasing the branch) to confirm the flake, and bumping `langchain-core` +together with `langchain` to fix the resolution conflict either way. + +Would you like me to post this as a comment on PR #921? + +## Case C — stale branch baseline drift (PR #906, pytest via Pipfile.lock) + +``` +| Field | Value | +|---|---| +| Package | pytest | +| Before version | 9.0.2 | +| Target version | 9.0.3 | +| CI checks | ❌ 9 failing (`smoke_test.yml`, all OS/Python matrix entries) | +| Actual installed version | Not verified (CI failed before this check) | +| Is covered (actual ≥ target) | — Not verified | +``` + +This PR only changes `Pipfile.lock`, which no workflow in this repo +actually installs from (no workflow runs `pipenv install`). The smoke +test failure is a behavior-count mismatch on a malware sample +(`Check Ahmyth Result` got 40, expected 39) — the kind of failure caused +by master adding new detection rules after this dependabot branch was +cut, not by a pytest/pygments/tomli version bump. + +**Recommendation: Needs human review** — suggest rebasing onto current +master and re-running before deciding. + +Would you like me to post this as a comment on PR #906? + +All three end with the explicit question. The skill never calls +`gh pr comment` or `gh pr merge` itself — it only posts if the human says +yes. diff --git a/.claude/skills/dependency-update-pr-validator/examples/ready_to_merge.md b/.claude/skills/dependency-update-pr-validator/examples/ready_to_merge.md new file mode 100644 index 000000000..677a147f0 --- /dev/null +++ b/.claude/skills/dependency-update-pr-validator/examples/ready_to_merge.md @@ -0,0 +1,28 @@ +# Example: Ready to merge + +Based on PR #923 (`ev-flow/quark-engine`), bumping `idna`. + +``` +| Field | Value | +|---|---| +| Package | idna | +| Before version | 3.11 | +| Target version | 3.15 | +| CI checks | ✅ All passed (push is skipping, not a failure) | +| Actual installed version | 3.15 | +| Is covered (actual ≥ target) | ✅ Yes | +``` + +CI ran the full test suite against idna 3.15 (confirmed via the +`Successfully installed ... idna-3.15 ...` line in the pytest.yml build +log) and passed. No workflow hardcodes an older idna version, so this +result reflects the target version, not a stale cache. + +**Recommendation: Ready to merge.** + +Would you like me to post this as a comment on PR #923? + +This is the full turn output — the table, the explanation, the +recommendation, then the explicit question. The skill never calls +`gh pr comment` or `gh pr merge` on its own; it only posts if the human +says yes to that last question. diff --git a/.claude/skills/dependency-update-pr-validator/scripts/check_actual_version.sh b/.claude/skills/dependency-update-pr-validator/scripts/check_actual_version.sh new file mode 100755 index 000000000..e7f17f1df --- /dev/null +++ b/.claude/skills/dependency-update-pr-validator/scripts/check_actual_version.sh @@ -0,0 +1,46 @@ +#!/usr/bin/env bash +# Find the actual installed version of a package, as proven by the `build` +# job log (pytest.yml), which is the only workflow that runs `pip install .` +# against this repo's real install_requires (setup.py). Pipfile/Pipfile.lock +# changes are NOT consumed by any current CI workflow — see SKILL.md. +# +# Usage: check_actual_version.sh +set -euo pipefail + +REPO="$1" +HEAD_REF="$2" +PACKAGE="$3" + +RUN_ID=$(gh run list --repo "$REPO" --workflow=pytest.yml --branch="$HEAD_REF" \ + --json databaseId,conclusion -q '.[0]') + +if [ -z "$RUN_ID" ] || [ "$RUN_ID" = "null" ]; then + echo "actual_version=UNKNOWN" + echo "reason=no pytest.yml run found for branch $HEAD_REF" + exit 0 +fi + +DB_ID=$(echo "$RUN_ID" | python3 -c 'import json,sys; print(json.load(sys.stdin)["databaseId"])') +CONCLUSION=$(echo "$RUN_ID" | python3 -c 'import json,sys; print(json.load(sys.stdin)["conclusion"])') + +echo "run_id=$DB_ID" +echo "run_conclusion=$CONCLUSION" + +if [ "$CONCLUSION" != "success" ]; then + echo "actual_version=UNKNOWN" + echo "reason=pytest.yml run did not succeed, install log is not a reliable signal" + exit 0 +fi + +VERSION=$(gh run view "$DB_ID" --repo "$REPO" --log 2>/dev/null \ + | grep -i "Successfully installed" \ + | grep -oE "(^|[[:space:]])${PACKAGE}-[0-9][0-9A-Za-z.+-]*" \ + | head -1 \ + | sed -E "s/^[[:space:]]*${PACKAGE}-//") + +if [ -z "$VERSION" ]; then + echo "actual_version=UNKNOWN" + echo "reason=package not found in 'Successfully installed' line (it may not be installed by pip install ., e.g. only declared in Pipfile)" +else + echo "actual_version=$VERSION" +fi diff --git a/.claude/skills/dependency-update-pr-validator/scripts/check_ci.sh b/.claude/skills/dependency-update-pr-validator/scripts/check_ci.sh new file mode 100755 index 000000000..94350317c --- /dev/null +++ b/.claude/skills/dependency-update-pr-validator/scripts/check_ci.sh @@ -0,0 +1,28 @@ +#!/usr/bin/env bash +# Summarize all status checks for a PR. A PR counts as "all passed" when +# every check is pass or skipping (skipping is normal — e.g. kali-package.yml +# only runs on push to master, so it's always skipped on a PR). +# +# Usage: check_ci.sh +set -euo pipefail + +REPO="$1" +PR="$2" + +CHECKS=$(gh pr checks "$PR" --repo "$REPO" 2>/dev/null) || true + +if [ -z "$CHECKS" ]; then + echo "all_passed=unknown" + echo "reason=no checks reported yet" + exit 0 +fi + +FAILING=$(echo "$CHECKS" | awk -F'\t' '$2 != "pass" && $2 != "skipping" {print}') + +if [ -z "$FAILING" ]; then + echo "all_passed=yes" +else + echo "all_passed=no" + echo "--- failing checks ---" + echo "$FAILING" +fi diff --git a/.claude/skills/dependency-update-pr-validator/scripts/check_pip_resolution.sh b/.claude/skills/dependency-update-pr-validator/scripts/check_pip_resolution.sh new file mode 100755 index 000000000..488d7bed5 --- /dev/null +++ b/.claude/skills/dependency-update-pr-validator/scripts/check_pip_resolution.sh @@ -0,0 +1,59 @@ +#!/usr/bin/env bash +# Only meaningful when the PR changed setup.py. A dependabot PR can bump one +# pin in a requirements list (e.g. quarkAgentRequirements) while leaving its +# sibling pins untouched — that combination can be mutually unsatisfiable +# even though pytest.yml's hardcoded install step never notices (it installs +# its own separate hardcoded versions, see check_workflow_pin.sh). This is a +# stronger, more definitive signal than is_covered: if pip itself can't +# resolve the post-PR setup.py, the bump is broken for any real user +# regardless of what CI happened to test. +# +# Scoped to just the requirement list that declares , so an +# unrelated pre-existing conflict elsewhere in setup.py doesn't get blamed +# on this PR. +# +# Usage: check_pip_resolution.sh +set -euo pipefail + +REPO="$1" +HEAD_REF="$2" +PACKAGE="$3" + +TMP_SETUP=$(mktemp) +trap 'rm -f "$TMP_SETUP"' EXIT + +gh api -H "Accept: application/vnd.github.raw" \ + "repos/$REPO/contents/setup.py?ref=$HEAD_REF" > "$TMP_SETUP" + +REQ_LINE=$(python3 - "$TMP_SETUP" "$PACKAGE" <<'PYEOF' +import ast, sys + +path, package = sys.argv[1], sys.argv[2] +with open(path) as f: + tree = ast.parse(f.read()) + +for node in ast.walk(tree): + if isinstance(node, ast.List): + items = [n.value for n in node.elts if isinstance(n, ast.Constant) and isinstance(n.value, str)] + if not items or not all(any(op in s for op in ("==", ">=", "<=")) for s in items): + continue + names = [s.split("==")[0].split(">=")[0].split("<=")[0].strip() for s in items] + if package in names: + print(" ".join(items)) + break +PYEOF +) + +if [ -z "$REQ_LINE" ]; then + echo "pip_resolution=skipped" + echo "reason=$PACKAGE not found in any pinned requirement list in setup.py" + exit 0 +fi + +echo "checking: $REQ_LINE" +if OUT=$(pip install --dry-run --quiet $REQ_LINE 2>&1); then + echo "pip_resolution=ok" +else + echo "pip_resolution=conflict" + echo "$OUT" | tail -15 +fi diff --git a/.claude/skills/dependency-update-pr-validator/scripts/check_workflow_pin.sh b/.claude/skills/dependency-update-pr-validator/scripts/check_workflow_pin.sh new file mode 100755 index 000000000..35f6d5a7b --- /dev/null +++ b/.claude/skills/dependency-update-pr-validator/scripts/check_workflow_pin.sh @@ -0,0 +1,26 @@ +#!/usr/bin/env bash +# Look for a hardcoded version pin of inside this repo's GitHub +# Actions workflow files (e.g. pytest.yml has a standalone +# `pip install langchain==0.2.11 langchain-core==0.2.23 langchain-openai==0.1.17` +# step that is NOT driven by setup.py/Pipfile). If such a pin exists, it +# silently overrides whatever version setup.py/Pipfile declares, so bumping +# the declared version alone will never change what CI actually installs. +# +# Checks the local working tree's workflow files (current master/branch +# state), which is a reasonable approximation since workflow files rarely +# change between a dependabot PR and master. +# +# Usage: check_workflow_pin.sh +set -euo pipefail + +PACKAGE="$1" +REPO_ROOT="$(git rev-parse --show-toplevel)" + +MATCHES=$(grep -rnE "(^|[^A-Za-z0-9_-])${PACKAGE}==[0-9]" "$REPO_ROOT/.github/workflows/" || true) + +if [ -z "$MATCHES" ]; then + echo "workflow_pin=none" +else + echo "workflow_pin=found" + echo "$MATCHES" +fi diff --git a/.claude/skills/dependency-update-pr-validator/scripts/compare_versions.sh b/.claude/skills/dependency-update-pr-validator/scripts/compare_versions.sh new file mode 100755 index 000000000..46b11dbb8 --- /dev/null +++ b/.claude/skills/dependency-update-pr-validator/scripts/compare_versions.sh @@ -0,0 +1,18 @@ +#!/usr/bin/env bash +# is_covered = actual installed version >= target version (PEP 440 semantics). +# +# Usage: compare_versions.sh +set -euo pipefail + +ACTUAL="$1" +TARGET="$2" + +python3 -c " +from packaging.version import Version +import sys +try: + print('yes' if Version('$ACTUAL') >= Version('$TARGET') else 'no') +except Exception as e: + print('unknown', file=sys.stderr) + sys.exit(1) +" diff --git a/.claude/skills/dependency-update-pr-validator/scripts/parse_pr.sh b/.claude/skills/dependency-update-pr-validator/scripts/parse_pr.sh new file mode 100755 index 000000000..607651937 --- /dev/null +++ b/.claude/skills/dependency-update-pr-validator/scripts/parse_pr.sh @@ -0,0 +1,42 @@ +#!/usr/bin/env bash +# Parse a dependabot dependency-update PR: package name, before/target +# version, and which files it touched. Which file changed (setup.py vs +# Pipfile/Pipfile.lock) does NOT tell you whether CI actually installs the +# target version — some setup.py-declared packages are overridden by a +# hardcoded pin elsewhere in the workflow (see check_workflow_pin.sh), and +# some Pipfile-only packages are unconstrained transitive deps that CI +# happily floats to the latest version anyway. Use check_actual_version.sh +# + check_workflow_pin.sh for that, not this script's changed_files. +# +# Usage: parse_pr.sh +set -euo pipefail + +REPO="$1" +PR="$2" + +TMP_BODY=$(mktemp) +trap 'rm -f "$TMP_BODY"' EXIT + +gh pr view "$PR" --repo "$REPO" --json body -q '.body' > "$TMP_BODY" +FILES=$(gh pr view "$PR" --repo "$REPO" --json files -q '.files[].path') +HEAD_REF=$(gh pr view "$PR" --repo "$REPO" --json headRefName -q '.headRefName') + +python3 - "$TMP_BODY" <<'PYEOF' +import re, sys + +with open(sys.argv[1]) as f: + body = f.read() + +m = re.search(r"Bumps \[([^\]]+)\].*?\bfrom\s+(\S+)\s+to\s+(\S+)\.", body, re.DOTALL) +if m: + print(f"package={m.group(1)}") + print(f"before_version={m.group(2)}") + print(f"target_version={m.group(3)}") +else: + print("package=UNKNOWN") + print("before_version=UNKNOWN") + print("target_version=UNKNOWN") +PYEOF + +echo "head_ref=$HEAD_REF" +echo "changed_files=$(echo "$FILES" | paste -sd, -)" diff --git a/.claude/skills/dependency-update-pr-validator/scripts/run_checkpoint.sh b/.claude/skills/dependency-update-pr-validator/scripts/run_checkpoint.sh new file mode 100755 index 000000000..0a2f04bc4 --- /dev/null +++ b/.claude/skills/dependency-update-pr-validator/scripts/run_checkpoint.sh @@ -0,0 +1,18 @@ +#!/usr/bin/env bash +# Prints the cheap checkpoint (no log digging): target package, before +# version, target version, and whether all CI checks passed. Run this first +# and show the result to the user before moving on to the slower step of +# digging through CI logs. +# +# Usage: run_checkpoint.sh +set -euo pipefail + +REPO="$1" +PR="$2" + +echo "=== PR info ===" +gh pr view "$PR" --repo "$REPO" --json title -q '.title' +"$(dirname "$0")/parse_pr.sh" "$REPO" "$PR" + +echo "=== CI status ===" +"$(dirname "$0")/check_ci.sh" "$REPO" "$PR" From a7dc38858a3cdd2561f81a91f40279a02ad31f75 Mon Sep 17 00:00:00 2001 From: haeter525 Date: Sat, 20 Jun 2026 04:47:16 +0000 Subject: [PATCH 2/5] docs(skills): clarify field labels, compress skill prose MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rename "Before version" -> "Current version" and "Is covered (actual ≥ target)" -> "Actual ≥ Target" in the skill's required output table and worked examples, and the matching parse_pr.sh output key (before_version -> current_version). Also compress SKILL.md prose to caveman style to cut input tokens on every load; code blocks and commands are untouched. Co-Authored-By: Claude Sonnet 4.6 --- .../dependency-update-pr-validator/SKILL.md | 193 +++++------------- .../examples/escalation_comment.md | 12 +- .../examples/ready_to_merge.md | 4 +- .../scripts/parse_pr.sh | 6 +- 4 files changed, 65 insertions(+), 150 deletions(-) diff --git a/.claude/skills/dependency-update-pr-validator/SKILL.md b/.claude/skills/dependency-update-pr-validator/SKILL.md index 64eb1503f..3b275811c 100644 --- a/.claude/skills/dependency-update-pr-validator/SKILL.md +++ b/.claude/skills/dependency-update-pr-validator/SKILL.md @@ -4,142 +4,79 @@ description: Validates dependabot dependency-update PRs against quark-engine's C version: 1.0.0 allowed-tools: bash(gh:*), bash(.claude/skills/dependency-update-pr-validator/scripts/*.sh) --- - # Dependency Update PR Validator -## Why this exists +## Why this exist -Dependabot opens a steady stream of near-identical, high-volume PRs against -`ev-flow/quark-engine` (one per dependency bump). Reviewing each one by hand -— does CI actually pass, does it actually test the new version — is -repetitive and easy to rubber-stamp. This skill automates that check and -produces a recommendation for a human to act on. See -[18z/QuarkHQ#3](https://github.com/18z/QuarkHQ/issues/3) for the original -ask. +Dependabot spam near-identical PRs at `ev-flow/quark-engine` (one per dep bump). Hand-review each — CI really pass? new version really tested? — repetitive, easy rubber-stamp. Skill automate check, spit recommendation for human act on. Origin ask: [18z/QuarkHQ#3](https://github.com/18z/QuarkHQ/issues/3). -**This skill never writes to GitHub.** It never calls `gh pr comment`, -`gh pr merge`, or `gh pr review`. Every output is a draft for a human to -read, edit, and post themselves — these PRs live in the upstream repo -(`ev-flow/quark-engine`), not the user's fork, so posting or merging there -is a visible, shared-state action that needs a human in the loop every -time. +**Skill never write GitHub.** Never call `gh pr comment`, `gh pr merge`, `gh pr review`. Every output draft for human read/edit/post self — PRs live upstream (`ev-flow/quark-engine`), not user fork, so post/merge there = visible shared-state action, need human loop every time. -## The core insight: green CI does not mean the target version was tested +## Core insight: green CI ≠ target version tested -This repo declares dependencies in two independent places that drift apart: +Repo declare deps two places, drift apart: - `setup.py` — actually consumed by `pip install .` in `pytest.yml`. -- `Pipfile` / `Pipfile.lock` — **not consumed by any current workflow.** - No workflow runs `pipenv install`. +- `Pipfile` / `Pipfile.lock` — **no workflow consume this.** Nothing run `pipenv install`. -On top of that, `pytest.yml` has a standalone line that hardcodes some -versions outright, e.g. (as of this writing): +Plus `pytest.yml` hardcode some versions outright, e.g. (as of writing): ``` python -m pip install langchain==0.2.11 langchain-core==0.2.23 langchain-openai==0.1.17 --upgrade ``` -That line ignores `setup.py` entirely. A dependabot PR that bumps -`langchain-core` in `setup.py` will still show all-green CI — CI just -silently kept testing the old pinned version. Conversely, fully unpinned -transitive deps (e.g. `idna`, `requests`) often float to whatever's latest -at install time regardless of which file dependabot touched, and that -floating version is frequently already at-or-above the dependabot target — -which is a legitimate pass, not a coincidence to be suspicious of. +Line ignore `setup.py` totally. Dependabot PR bump `langchain-core` in `setup.py` → still all-green CI — CI quietly kept testing old pinned version. Other way: unpinned transitive deps (e.g. `idna`, `requests`) often float to latest at install regardless of which file dependabot touch, and float version frequent already at-or-above target — legit pass, not coincidence to suspect. -The lesson: **don't infer anything from which file the PR changed.** -Always directly check (a) what version actually got installed in the CI -run, and (b) whether some workflow step hardcodes this exact package to an -older version. Those two checks are simple greps; trust them over the -filename. +Lesson: **don't guess from which file PR changed.** Always check direct (a) what version actually install in CI run, (b) does some workflow step hardcode this exact package to older version. Two simple greps — trust over filename. ## Workflow -Run each step and **print its result before moving to the next one** — -the slow part is digging through CI logs, so show the cheap facts -(package, before/target version, CI pass/fail) first rather than holding -everything for one final report. +Run each step, **print result before next step** — slow part dig CI logs, so show cheap facts (package, current/target version, CI pass/fail) first, not hold all for one final report. -### Step 1 — Parse the PR (cheap, do this first) +### Step 1 — Parse PR (cheap, do first) ```bash bash .claude/skills/dependency-update-pr-validator/scripts/run_checkpoint.sh ``` -This prints the PR title, package name, before version, target version, -and whether every CI check passed (treating `skipping` as fine — e.g. -`kali-package.yml` only runs on push to master, so it's always skipped on -a PR; that is not a failure). +Print PR title, package name, current version, target version, all CI checks passed (treat `skipping` as fine — e.g. `kali-package.yml` only run on push to master, always skip on PR; not failure). -Show this checkpoint to the user immediately. +Show checkpoint to user now. -### Step 2 — If `setup.py` was changed: confirm the new pin is actually installable +### Step 2 — If `setup.py` changed: confirm new pin actually installable -Run this whenever `setup.py` is among the PR's changed files (check -`changed_files` from Step 1) — **regardless of CI pass/fail**. This is a -static check of the PR's own diff, not of what CI happened to test, so a -CI failure in Step 3 doesn't make it irrelevant — a PR can fail CI for an -unrelated reason *and* be independently broken here, and you want to catch -both. +Run whenever `setup.py` among PR's changed files (check `changed_files` from Step 1) — **regardless CI pass/fail**. Static check of PR's own diff, not what CI happen test — CI failure Step 3 don't make this irrelevant — PR can fail CI for unrelated reason *and* be independently broken here, want catch both. ```bash bash .claude/skills/dependency-update-pr-validator/scripts/check_pip_resolution.sh ``` -A dependabot PR usually bumps one pin inside a requirements list (e.g. -`quarkAgentRequirements` in `setup.py`) while leaving its sibling pins in -the same list untouched. That combination can be mutually unsatisfiable — -`pip install` for those exact pinned versions together raises -`ResolutionImpossible` — even when CI is fully green, because CI's -hardcoded install step (Step 4) tests its own separate versions and never -actually tries to install what `setup.py` now declares. This is a -stronger, more definitive reason to block a merge than `is_covered`: if -real users can't even `pip install` the result, it doesn't matter what CI -tested. If this check reports `pip_resolution=conflict`, the recommendation -is **always** "Needs human review," regardless of what CI/is_covered says. - -### Step 3 — If any check failed: find the real root cause, don't just report "CI failed" - -A red check on a dependabot PR is frequently **not caused by the bump at -all**. Two patterns observed in practice (see -[examples/escalation_comment.md](examples/escalation_comment.md) for full -writeups): - -- **Environment flake** — a test fixture fails to download a valid file - (e.g. `zipfile.BadZipFile: File is not a zip file` while fetching a - sample APK). Unrelated to any dependency. -- **Stale-branch baseline drift** — `smoke_test.yml`'s hardcoded expected - behavior-counts (e.g. "Ahmyth.apk should show 39 behaviors") no longer - match because master gained new detection rules after the dependabot - branch was cut. Also unrelated to the dependency bump. - -To find the actual cause: +Dependabot PR usually bump one pin inside requirements list (e.g. `quarkAgentRequirements` in `setup.py`) while leave sibling pins same list untouched. Combo can be mutually unsatisfiable — `pip install` for those exact pinned versions together raise `ResolutionImpossible` — even when CI fully green, cuz CI's hardcoded install step (Step 4) test own separate versions, never actually try install what `setup.py` now declare. Stronger, more definitive reason block merge than `is_covered`: if real users can't even `pip install` result, don't matter what CI tested. If check report `pip_resolution=conflict`, recommendation **always** "Needs human review," regardless what CI/is_covered say. + +### Step 3 — If any check failed: find real root cause, don't just report "CI failed" + +Red check on dependabot PR frequent **not caused by bump at all**. Two pattern seen practice (see [examples/escalation_comment.md](examples/escalation_comment.md) for full writeups): + +- **Environment flake** — test fixture fail download valid file (e.g. `zipfile.BadZipFile: File is not a zip file` fetching sample APK). Unrelated any dependency. +- **Stale-branch baseline drift** — `smoke_test.yml`'s hardcoded expected behavior-counts (e.g. "Ahmyth.apk should show 39 behaviors") no longer match cuz master gain new detection rules after dependabot branch cut. Also unrelated bump. + +Find actual cause: ```bash gh run view --repo --log-failed ``` -Read the failure, classify it as "looks like a real regression from this -bump" vs. "looks unrelated" (give your reasoning — e.g. the failing -package is a test-only tool that can't plausibly affect malware-detection -counts), and stop here (skip Step 4 — there's no successful build log to -read a version out of). This is always an escalation regardless of -classification — only difference is what you tell the human, and Step 2's -finding (if any) still applies on top of it. +Read failure, classify "look like real regression from bump" vs "look unrelated" (give reasoning — e.g. failing package test-only tool, can't plausibly affect malware-detection counts), stop here (skip Step 4 — no successful build log read version from). Always escalation regardless classification — only diff what tell human, and Step 2's finding (if any) still apply on top. -### Step 4 — If CI passed: check what was actually installed +### Step 4 — If CI passed: check what actually installed ```bash bash .claude/skills/dependency-update-pr-validator/scripts/check_actual_version.sh ``` -This finds the `pytest.yml` ("build") run for the PR's branch and greps -its log for the package's `Successfully installed` line — the one place -that reflects what pip genuinely resolved and tested, regardless of which -declaration file the PR touched. +Find `pytest.yml` ("build") run for PR's branch, grep log for package's `Successfully installed` line — one place reflect what pip genuinely resolve+test, regardless which declaration file PR touch. -Then check whether a workflow hardcodes this package to a stale version -(this is what explains a "No" result, when there is one): +Then check workflow hardcode package to stale version (explains "No" result, when exist): ```bash bash .claude/skills/dependency-update-pr-validator/scripts/check_workflow_pin.sh @@ -151,74 +88,52 @@ Then compute is_covered: bash .claude/skills/dependency-update-pr-validator/scripts/compare_versions.sh ``` -### Step 5 — Print the result table, then ask before posting anything +### Step 5 — Print result table, then ask before posting anything -Always finish with **one markdown table in English**, in this exact field -order, regardless of which branch you took: +Always finish **one markdown table English**, exact field order, regardless which branch taken: ``` | Field | Value | |---|---| | Package | | -| Before version | | +| Current version | | | Target version | | | CI checks | <✅ All passed / ❌ N failing, with a short reason> | | Actual installed version | | -| Is covered (actual ≥ target) | <✅ Yes / ❌ No / — Not verified> | +| Actual ≥ Target | <✅ Yes / ❌ No / — Not verified> | ``` -Right after the table, add whatever prose is needed to explain *why* — -quote the hardcoded pin line if `check_workflow_pin.sh` found one, quote -the `pip install` conflict if `check_pip_resolution.sh` reported one, or -give the root-cause classification from Step 3 if CI failed — then one -line: **Recommendation: Ready to merge.** or **Recommendation: Needs human -review.** A `pip_resolution=conflict` result always forces "Needs human -review," even if CI passed and is_covered is Yes. See -[examples/ready_to_merge.md](examples/ready_to_merge.md) and -[examples/escalation_comment.md](examples/escalation_comment.md) for full -worked examples in this exact format. - -Then **ask the user**, every time, whether to post this as a comment on -the PR — e.g. "Would you like me to post this as a comment on PR #923?". -Only call `gh pr comment --repo --body "..."` if they say yes. -Never post automatically, and never call `gh pr merge` regardless of the -recommendation — merging is the human's call to make after reading this. - -Present the draft to the user. Do not post it anywhere. +Right after table, add prose explain *why* — quote hardcoded pin line if `check_workflow_pin.sh` found one, quote `pip install` conflict if `check_pip_resolution.sh` reported one, or give root-cause classification from Step 3 if CI failed — then one line: **Recommendation: Ready to merge.** or **Recommendation: Needs human review.** `pip_resolution=conflict` result always force "Needs human review," even CI passed and is_covered Yes. See [examples/ready_to_merge.md](examples/ready_to_merge.md) and [examples/escalation_comment.md](examples/escalation_comment.md) full worked examples this exact format. + +Then **ask user**, every time, post this as comment on PR? — e.g. "Would you like me to post this as a comment on PR #923?". Only call `gh pr comment --repo --body "..."` if yes say. Never post auto, never call `gh pr merge` regardless recommendation — merge human's call after read this. + +Present draft to user. Don't post anywhere. ## Scripts reference | Script | Purpose | |---|---| -| `scripts/run_checkpoint.sh ` | Step 1: package, before/target version, CI pass/fail | -| `scripts/parse_pr.sh ` | Just the dependabot body/diff parsing part of step 1 | -| `scripts/check_ci.sh ` | Just the CI-checks part of step 1 | -| `scripts/check_pip_resolution.sh ` | Step 2 (only if `setup.py` changed, regardless of CI outcome): is the bumped pin pip-installable alongside its sibling pins in the same requirements list? | -| `scripts/check_actual_version.sh ` | Step 4: actual installed version from the pytest.yml build log | -| `scripts/check_workflow_pin.sh ` | Step 4: does a workflow hardcode this package to an older version? | +| `scripts/run_checkpoint.sh ` | Step 1: package, current/target version, CI pass/fail | +| `scripts/parse_pr.sh ` | Just dependabot body/diff parse part step 1 | +| `scripts/check_ci.sh ` | Just CI-checks part step 1 | +| `scripts/check_pip_resolution.sh ` | Step 2 (only if `setup.py` changed, regardless CI outcome): bumped pin pip-installable alongside sibling pins same requirements list? | +| `scripts/check_actual_version.sh ` | Step 4: actual installed version from pytest.yml build log | +| `scripts/check_workflow_pin.sh ` | Step 4: workflow hardcode package to older version? | | `scripts/compare_versions.sh ` | Step 4: is_covered (PEP 440 version comparison) | -All scripts default to the real PR repo when you pass `ev-flow/quark-engine` -as `` — that's where every dependabot PR for this project lives, not -the user's fork. +All scripts default to real PR repo when pass `ev-flow/quark-engine` +as `` — that's where every dependabot PR this project live, not user fork. ## Validated against -This logic was walked through manually against five real dependabot PRs -before being written down here — keep these as a sanity check if you -change the logic: +Logic walked manual against five real dependabot PRs before write down here — keep as sanity check if change logic: | PR | Package | Outcome | |---|---|---| -| [#923](https://github.com/ev-flow/quark-engine/pull/923) | idna | Ready to merge — CI green, actual version matches target, no pin found | -| [#922](https://github.com/ev-flow/quark-engine/pull/922) | langchain-core | Needs human — CI green but `pytest.yml:53` pins an older version; target never installed. Also: `pip install` for the post-PR `setup.py` (`langchain==0.2.11 langchain-core==1.3.3 langchain-openai==0.1.17`) raises `ResolutionImpossible` — broken regardless of CI | -| [#921](https://github.com/ev-flow/quark-engine/pull/921) | langchain | Needs human — CI red, root cause is an unrelated sample-download flake. Also independently fails `check_pip_resolution.sh`: `langchain==0.3.30` conflicts with the untouched sibling pin `langchain-core==0.2.23` — broken even after a clean re-run | -| [#906](https://github.com/ev-flow/quark-engine/pull/906) | pytest (Pipfile.lock) | Needs human — CI red, but root cause is smoke-test baseline drift from a stale branch, unrelated to pytest | -| [#893](https://github.com/ev-flow/quark-engine/pull/893) | requests (Pipfile.lock) | Ready to merge — CI green, actual version (resolved transitively) exceeds target, no pin found | - -The `check_pip_resolution.sh` step (and the `ResolutionImpossible` finding -for #922 above) was added after an eval comparison against a baseline -agent without this skill — it independently ran `pip install` on the -post-PR `setup.py` and caught a more definitive, CI-independent reason to -block the PR. See `evals/` in this skill directory for the eval set this -was validated against. +| [#923](https://github.com/ev-flow/quark-engine/pull/923) | idna | Ready to merge — CI green, actual version match target, no pin found | +| [#922](https://github.com/ev-flow/quark-engine/pull/922) | langchain-core | Needs human — CI green but `pytest.yml:53` pin older version; target never install. Also: `pip install` for post-PR `setup.py` (`langchain==0.2.11 langchain-core==1.3.3 langchain-openai==0.1.17`) raise `ResolutionImpossible` — broken regardless CI | +| [#921](https://github.com/ev-flow/quark-engine/pull/921) | langchain | Needs human — CI red, root cause unrelated sample-download flake. Also independently fail `check_pip_resolution.sh`: `langchain==0.3.30` conflict untouched sibling pin `langchain-core==0.2.23` — broken even after clean re-run | +| [#906](https://github.com/ev-flow/quark-engine/pull/906) | pytest (Pipfile.lock) | Needs human — CI red, but root cause smoke-test baseline drift from stale branch, unrelated pytest | +| [#893](https://github.com/ev-flow/quark-engine/pull/893) | requests (Pipfile.lock) | Ready to merge — CI green, actual version (resolved transitively) exceed target, no pin found | + +`check_pip_resolution.sh` step (and `ResolutionImpossible` finding for #922 above) added after eval comparison vs baseline agent without this skill — it independently ran `pip install` on post-PR `setup.py`, caught more definitive, CI-independent reason block PR. See `evals/` this skill dir for eval set validated against. diff --git a/.claude/skills/dependency-update-pr-validator/examples/escalation_comment.md b/.claude/skills/dependency-update-pr-validator/examples/escalation_comment.md index 61201aaaf..663906210 100644 --- a/.claude/skills/dependency-update-pr-validator/examples/escalation_comment.md +++ b/.claude/skills/dependency-update-pr-validator/examples/escalation_comment.md @@ -8,11 +8,11 @@ CI was green, but the green result is misleading. | Field | Value | |---|---| | Package | langchain-core | -| Before version | 0.2.23 | +| Current version | 0.2.23 | | Target version | 1.3.3 | | CI checks | ✅ All passed (push is skipping, not a failure) | | Actual installed version | 0.2.23 (unchanged!) | -| Is covered (actual ≥ target) | ❌ No | +| Actual ≥ Target | ❌ No | ``` `pytest.yml` line 53 hardcodes @@ -49,11 +49,11 @@ Would you like me to post this as a comment on PR #922? | Field | Value | |---|---| | Package | langchain | -| Before version | 0.2.11 | +| Current version | 0.2.11 | | Target version | 0.3.30 | | CI checks | ❌ 1 failing (`build`, pytest.yml) | | Actual installed version | Not verified (CI failed before this check) | -| Is covered (actual ≥ target) | — Not verified | +| Actual ≥ Target | — Not verified | ``` All 461 test errors trace back to one fixture failing to download a valid @@ -83,11 +83,11 @@ Would you like me to post this as a comment on PR #921? | Field | Value | |---|---| | Package | pytest | -| Before version | 9.0.2 | +| Current version | 9.0.2 | | Target version | 9.0.3 | | CI checks | ❌ 9 failing (`smoke_test.yml`, all OS/Python matrix entries) | | Actual installed version | Not verified (CI failed before this check) | -| Is covered (actual ≥ target) | — Not verified | +| Actual ≥ Target | — Not verified | ``` This PR only changes `Pipfile.lock`, which no workflow in this repo diff --git a/.claude/skills/dependency-update-pr-validator/examples/ready_to_merge.md b/.claude/skills/dependency-update-pr-validator/examples/ready_to_merge.md index 677a147f0..f2ce8fb2b 100644 --- a/.claude/skills/dependency-update-pr-validator/examples/ready_to_merge.md +++ b/.claude/skills/dependency-update-pr-validator/examples/ready_to_merge.md @@ -6,11 +6,11 @@ Based on PR #923 (`ev-flow/quark-engine`), bumping `idna`. | Field | Value | |---|---| | Package | idna | -| Before version | 3.11 | +| Current version | 3.11 | | Target version | 3.15 | | CI checks | ✅ All passed (push is skipping, not a failure) | | Actual installed version | 3.15 | -| Is covered (actual ≥ target) | ✅ Yes | +| Actual ≥ Target | ✅ Yes | ``` CI ran the full test suite against idna 3.15 (confirmed via the diff --git a/.claude/skills/dependency-update-pr-validator/scripts/parse_pr.sh b/.claude/skills/dependency-update-pr-validator/scripts/parse_pr.sh index 607651937..0d1f99746 100755 --- a/.claude/skills/dependency-update-pr-validator/scripts/parse_pr.sh +++ b/.claude/skills/dependency-update-pr-validator/scripts/parse_pr.sh @@ -1,5 +1,5 @@ #!/usr/bin/env bash -# Parse a dependabot dependency-update PR: package name, before/target +# Parse a dependabot dependency-update PR: package name, current/target # version, and which files it touched. Which file changed (setup.py vs # Pipfile/Pipfile.lock) does NOT tell you whether CI actually installs the # target version — some setup.py-declared packages are overridden by a @@ -30,11 +30,11 @@ with open(sys.argv[1]) as f: m = re.search(r"Bumps \[([^\]]+)\].*?\bfrom\s+(\S+)\s+to\s+(\S+)\.", body, re.DOTALL) if m: print(f"package={m.group(1)}") - print(f"before_version={m.group(2)}") + print(f"current_version={m.group(2)}") print(f"target_version={m.group(3)}") else: print("package=UNKNOWN") - print("before_version=UNKNOWN") + print("current_version=UNKNOWN") print("target_version=UNKNOWN") PYEOF From a8ff16f47363adac821655ff36c3c9150a04913b Mon Sep 17 00:00:00 2001 From: haeter525 Date: Sat, 20 Jun 2026 04:48:49 +0000 Subject: [PATCH 3/5] docs(skills): un-stale the workflow-pin example in SKILL.md The pytest.yml hardcoded langchain pin this section described was fixed in 9d119c4, but the doc still stated it as a present-tense fact. Reframe as a history lesson explaining why check_workflow_pin.sh still matters (a different package could grow a new hardcoded pin later) instead of describing dead code. Co-Authored-By: Claude Sonnet 4.6 --- .claude/skills/dependency-update-pr-validator/SKILL.md | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/.claude/skills/dependency-update-pr-validator/SKILL.md b/.claude/skills/dependency-update-pr-validator/SKILL.md index 3b275811c..c8b8ce932 100644 --- a/.claude/skills/dependency-update-pr-validator/SKILL.md +++ b/.claude/skills/dependency-update-pr-validator/SKILL.md @@ -16,16 +16,12 @@ Dependabot spam near-identical PRs at `ev-flow/quark-engine` (one per dep bump). Repo declare deps two places, drift apart: -- `setup.py` — actually consumed by `pip install .` in `pytest.yml`. +- `setup.py` — consumed by `pip install ".[QuarkAgent]"` in `pytest.yml`. - `Pipfile` / `Pipfile.lock` — **no workflow consume this.** Nothing run `pipenv install`. -Plus `pytest.yml` hardcode some versions outright, e.g. (as of writing): +History lesson (fixed, but why `check_workflow_pin.sh` still exists): `pytest.yml` used to have a standalone line hardcoding `langchain==0.2.11 langchain-core==0.2.23 langchain-openai==0.1.17`, totally ignoring `setup.py`. Dependabot bump `langchain-core` in `setup.py` → still all-green CI, cuz CI quietly kept testing the old hardcoded version, never the bump. Fixed by installing `.[QuarkAgent]` extras instead (commit `9d119c4`) — but **any** workflow step could grow a new hardcoded pin like this again for some other package, so still check for it, don't assume it's gone forever. -``` -python -m pip install langchain==0.2.11 langchain-core==0.2.23 langchain-openai==0.1.17 --upgrade -``` - -Line ignore `setup.py` totally. Dependabot PR bump `langchain-core` in `setup.py` → still all-green CI — CI quietly kept testing old pinned version. Other way: unpinned transitive deps (e.g. `idna`, `requests`) often float to latest at install regardless of which file dependabot touch, and float version frequent already at-or-above target — legit pass, not coincidence to suspect. +Separate, still-live gotcha: unpinned transitive deps (e.g. `idna`, `requests`) often float to latest at install regardless of which file dependabot touch, and float version frequent already at-or-above target — legit pass, not coincidence to suspect. Lesson: **don't guess from which file PR changed.** Always check direct (a) what version actually install in CI run, (b) does some workflow step hardcode this exact package to older version. Two simple greps — trust over filename. From d06f4e445e77bf1fb33e2510972ad028c354e774 Mon Sep 17 00:00:00 2001 From: haeter525 Date: Tue, 23 Jun 2026 17:09:42 +0000 Subject: [PATCH 4/5] docs(skills): refine dependency-update PR validator guidance Clarify SKILL.md wording, update the ready-to-merge example, and adjust check_actual_version.sh to report the installed version accurately. Co-Authored-By: Claude Opus 4.8 --- .claude/skills/dependency-update-pr-validator/SKILL.md | 6 +++--- .../examples/ready_to_merge.md | 6 ++++-- .../scripts/check_actual_version.sh | 5 +++++ 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/.claude/skills/dependency-update-pr-validator/SKILL.md b/.claude/skills/dependency-update-pr-validator/SKILL.md index c8b8ce932..df2df4b31 100644 --- a/.claude/skills/dependency-update-pr-validator/SKILL.md +++ b/.claude/skills/dependency-update-pr-validator/SKILL.md @@ -70,7 +70,7 @@ Read failure, classify "look like real regression from bump" vs "look unrelated" bash .claude/skills/dependency-update-pr-validator/scripts/check_actual_version.sh ``` -Find `pytest.yml` ("build") run for PR's branch, grep log for package's `Successfully installed` line — one place reflect what pip genuinely resolve+test, regardless which declaration file PR touch. +Find `pytest.yml` ("build") run for PR's branch, grep log for package's `Successfully installed` line — one place reflect what pip genuinely resolve+test, regardless which declaration file PR touch. Script also emit `proof_url` (link to the `build` job page) — link this in table row as "Found in CI log", not just bare version, so human verify same place without re-running anything. Then check workflow hardcode package to stale version (explains "No" result, when exist): @@ -95,7 +95,7 @@ Always finish **one markdown table English**, exact field order, regardless whic | Current version | | | Target version | | | CI checks | <✅ All passed / ❌ N failing, with a short reason> | -| Actual installed version | | +| Actual installed version | | | Actual ≥ Target | <✅ Yes / ❌ No / — Not verified> | ``` @@ -113,7 +113,7 @@ Present draft to user. Don't post anywhere. | `scripts/parse_pr.sh ` | Just dependabot body/diff parse part step 1 | | `scripts/check_ci.sh ` | Just CI-checks part step 1 | | `scripts/check_pip_resolution.sh ` | Step 2 (only if `setup.py` changed, regardless CI outcome): bumped pin pip-installable alongside sibling pins same requirements list? | -| `scripts/check_actual_version.sh ` | Step 4: actual installed version from pytest.yml build log | +| `scripts/check_actual_version.sh ` | Step 4: actual installed version from pytest.yml build log, plus `proof_url` link to that job | | `scripts/check_workflow_pin.sh ` | Step 4: workflow hardcode package to older version? | | `scripts/compare_versions.sh ` | Step 4: is_covered (PEP 440 version comparison) | diff --git a/.claude/skills/dependency-update-pr-validator/examples/ready_to_merge.md b/.claude/skills/dependency-update-pr-validator/examples/ready_to_merge.md index f2ce8fb2b..ce3526b36 100644 --- a/.claude/skills/dependency-update-pr-validator/examples/ready_to_merge.md +++ b/.claude/skills/dependency-update-pr-validator/examples/ready_to_merge.md @@ -9,14 +9,16 @@ Based on PR #923 (`ev-flow/quark-engine`), bumping `idna`. | Current version | 3.11 | | Target version | 3.15 | | CI checks | ✅ All passed (push is skipping, not a failure) | -| Actual installed version | 3.15 | +| Actual installed version | 3.15 ([Found in CI log](https://github.com/ev-flow/quark-engine/actions/runs/24300403080/job/70952658255)) | | Actual ≥ Target | ✅ Yes | ``` CI ran the full test suite against idna 3.15 (confirmed via the `Successfully installed ... idna-3.15 ...` line in the pytest.yml build log) and passed. No workflow hardcodes an older idna version, so this -result reflects the target version, not a stale cache. +result reflects the target version, not a stale cache. The proof link +points straight at the `build` job log, so a human can verify the +install line without re-running anything. **Recommendation: Ready to merge.** diff --git a/.claude/skills/dependency-update-pr-validator/scripts/check_actual_version.sh b/.claude/skills/dependency-update-pr-validator/scripts/check_actual_version.sh index e7f17f1df..09515071f 100755 --- a/.claude/skills/dependency-update-pr-validator/scripts/check_actual_version.sh +++ b/.claude/skills/dependency-update-pr-validator/scripts/check_actual_version.sh @@ -26,6 +26,11 @@ CONCLUSION=$(echo "$RUN_ID" | python3 -c 'import json,sys; print(json.load(sys.s echo "run_id=$DB_ID" echo "run_conclusion=$CONCLUSION" +JOB_ID=$(gh run view "$DB_ID" --repo "$REPO" --json jobs -q '.jobs[] | select(.name=="build") | .databaseId' | head -1) +if [ -n "$JOB_ID" ]; then + echo "proof_url=https://github.com/${REPO}/actions/runs/${DB_ID}/job/${JOB_ID}" +fi + if [ "$CONCLUSION" != "success" ]; then echo "actual_version=UNKNOWN" echo "reason=pytest.yml run did not succeed, install log is not a reliable signal" From 6ce23a7c3656f5dee6fe3d61bd1f0ee07b125c9f Mon Sep 17 00:00:00 2001 From: haeter525 Date: Wed, 1 Jul 2026 16:41:33 +0000 Subject: [PATCH 5/5] fix(skills): scope allowed-tools to read-only gh subcommands bash(gh:*) pre-authorized all gh subcommands including gh pr comment, gh pr merge, and gh pr review. Replaced with a scoped allowlist of the read-only verbs the scripts actually use, so any GitHub write requires an explicit human permission prompt. Co-Authored-By: Claude Sonnet 4.6 --- .claude/skills/dependency-update-pr-validator/SKILL.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/.claude/skills/dependency-update-pr-validator/SKILL.md b/.claude/skills/dependency-update-pr-validator/SKILL.md index df2df4b31..c808c9819 100644 --- a/.claude/skills/dependency-update-pr-validator/SKILL.md +++ b/.claude/skills/dependency-update-pr-validator/SKILL.md @@ -2,7 +2,13 @@ name: dependency-update-pr-validator description: Validates dependabot dependency-update PRs against quark-engine's CI and drafts a merge/escalate recommendation. Use when asked to review, validate, or triage a dependabot PR on ev-flow/quark-engine, or when the user references issue 18z/QuarkHQ#3. version: 1.0.0 -allowed-tools: bash(gh:*), bash(.claude/skills/dependency-update-pr-validator/scripts/*.sh) +allowed-tools: >- + bash(gh pr view:*), + bash(gh pr checks:*), + bash(gh run list:*), + bash(gh run view:*), + bash(gh api:*), + bash(.claude/skills/dependency-update-pr-validator/scripts/*.sh) --- # Dependency Update PR Validator