Repository navigation
Conversation
… + confirmed plugins
An earlier draft added options { ansiColor('xterm') } which failed on the
target Jenkins controller with "invalid option type \"ansiColor\"" since the
AnsiColor plugin isn't installed. Add references/eea-available-plugins.md
(sourced from eea/eea.docker.jenkins.master/plugins.txt) as the allow-list,
and a rule in EEA-OVERRIDES.md requiring every non-core step/options entry
to be checked against it before use.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…host agents EEA docker-host Jenkins agents run Docker-outside-of-Docker: only the docker socket is forwarded, not the filesystem, so -v "$WORKSPACE:/path" mounts resolve to an empty directory on the real dockerd host. This is what broke ruff/black/mypy in the Code linting stage with "No such file or directory" for every path. Document the constraint in EEA-OVERRIDES.md and fix the Auto-fix code style stage in eea-jenkinsfile-template.md to use docker cp against the image's baked-in source instead of a workspace bind mount. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…gnore HIGH,CRITICAL was too strict: a real scan of a python:3.11-slim release image turned up 33 HIGH findings in base-OS packages outside the repo owner's control. Gate the build on CRITICAL only; still archive a HIGH,CRITICAL report so HIGH stays visible. Also document how to get .trivyignore into the trivy scan: it lives in the checked-out workspace, which the trivy container can't see via a bind mount (same Docker-outside-of-Docker constraint as other stages), so it needs docker create + docker cp + docker start instead of -v. Validated end to end against a real scan (4 CRITICAL findings, all unfixed-upstream perl-base CVEs on Debian trixie) — confirmed the ignorefile suppresses them and the gate exits 0. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…rfiles Extend docker-expert to scan the release image locally for CRITICAL CVEs before considering a production Dockerfile finished, mirroring the jenkins-pipeline CRITICAL-only gate: bump versions when a fix exists, add to .trivyignore with a reason when it doesn't (never blanket-ignore a severity tier). Cross-reference this from jenkins-pipeline's EEA overrides so it runs before the Trivy stage is finalized, the same way the existing lint/test preflight already works — so a Jenkins run isn't the first time anyone learns about an unresolved CRITICAL CVE. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Document that Python coverage must go to sonar.python.coverage.reportPaths as Cobertura XML, not sonar.javascript.lcov.reportPaths — a real bug where Python-generated LCOV was wired into the JS property, silently producing 0% coverage in Sonar despite tests passing and coverage being measured correctly. Also document the Cobertura multi-root pitfall: passing several disjoint --cov=<dir> roots makes coverage.py's XML writer emit ambiguous bare filenames (no directory prefix) unlike its LCOV writer, which stays correctly qualified — use --cov=. plus a source/omit config instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…path gotchas Two more real bugs found while debugging the mission-aipossible pipeline: - docker cp <container>:<dir> <dest> nests instead of copying contents when <dest> already exists (common here, since earlier stages mkdir -p their output dirs) — always use a trailing /. on the source to get contents regardless of destination state. - sonar.python.xunit.reportPath is a single path (wildcards supported), not a comma-delimited list like the coverage property — comma-joining two junit.xml paths gets treated as one literal pattern matching neither file. Use a wildcard glob that matches both instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… conversation Replace the previous badge guidance (which invented a false volto-addons vs EEA-AI distinction and an unverified "2 badges only" claim) with a single convention: never add badges without asking, and even then ask which specific badges and branch(es) rather than applying a template. Documents the confirmed Jenkins pipeline badge URL pattern (org-folder is project-specific, e.g. EEA-AI for AI applications) and the 6 canonical SonarQube measure badges (Coverage, Duplications, Security Hotspots Reviewed, Maintainability, Reliability, Security) with their metric keys — verified against SonarQube's own badge API metric list rather than guessed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…lto-addon-template ref Remove the last volto-addon-template reference (a general Jenkinsfile pattern pointer, unrelated to badges but the org's convention no longer wants it cited at all). Change badge guidance from "wait for the developer to ask" to "ask proactively once the Jenkinsfile is finalized": a developer setting up Jenkins for the first time won't know EEA has a badge convention, but badges can't be filled in with a working URL until the Jenkins job path and Sonar project key exist, which only happens once the pipeline itself is written. The skill now treats this as a required post-Jenkinsfile step, not an optional response to an unprompted question. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…r project shapes
This skill's default template assumes a Docker-based JS/Python app with
its own Dockerfile.test — a real gap for the other project shapes EEA
actually has in production. Add references/examples/, one file per shape,
each with the source repo/branch (EEA archives rather than deletes repos,
so these stay fetchable for old projects), a "when to reach for this one"
section, and the full real Jenkinsfile:
- python-egg-plone-jenkinsfile.md (eea.genai.blocks): PyPI/egg release,
per-tool eeacms/<tool> images instead of a project test image.
- java-maven-jenkinsfile.md (eionet.xmlconv): Maven tools{} instead of
Dockerfile.test, JaCoCo coverage, mvn sonar:sonar.
- nodejs-volto-jenkinsfile.md (volto-addon-template): multi-version Volto
test matrix, Cypress integration against a real Plone backend,
SonarQube-compare-to-master quality gate.
- docker-helm-release-jenkinsfile.md (eea.docker.jenkins.master): minimal
tag-triggered release-only pipeline for a Dockerfile + Helm/Rancher
catalog template repo with nothing to test.
All four use eeacms/gitflow for release automation regardless of
language/stack. Wired into the Required workflow (check examples/ before
assuming the default template fits) and a new EEA-OVERRIDES.md section
indexing all four with when to reach for each.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…Quality Gate guidance sonar.pullrequest.base/branch/key alone don't make SonarQube post anything back to GitHub — that requires binding the project to a GitHub DevOps Platform Integration (GitHubEEA at EEA), done via api/alm_settings/set_github_binding (verified against SonarQube's own Java WS client source: almSetting/project/repository required, summaryCommentEnabled/monorepo optional). Document it as an idempotent curl call alongside the existing sonar-scanner invocation, same idiom as the existing api/project_tags/set calls in these pipelines. Also: the Quality Gate badge is a separate endpoint (api/project_badges/quality_gate, no metric= param) from the 6 measure badges already documented — added as a 7th badge option. And a short recommendation: gate on New Code conditions (Clean as You Code) rather than absolute thresholds, and make the GitHub check a required status once PR decoration works. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…Path is non-functional Walk back the earlier "use a wildcard glob" fix: tested on a real Jenkins PR build and got the identical "No report was found" warning even with junit.xml provably present at the exact configured path (confirmed via the ls -la diagnostic immediately before the scan step). Two different values failing identically against files known to exist means this sensor doesn't respect custom values in current SonarQube versions, not a pattern-syntax issue worth continuing to chase. Recommend dropping the property entirely: Jenkins' own junit step already publishes per-test results to the GitHub check, and the Quality Gate is driven by coverage + static analysis, not this sensor. Same reasoning applies to sonar.junit.reportPaths (Java-oriented, no-op without a Java sensor). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…red sequence The previous workflow described the right ideas (build Dockerfile.test, preflight quality/test commands, ask before generating the Jenkinsfile) but as loosely-ordered prose an agent could reasonably skip or reorder — confirmed by a from-scratch test session that generated a Jenkinsfile without actually building Dockerfile.test and running the real commands against it first. Restructure into 4 explicit phases: Discover, Build the exact image CI will run in, Preflight with the exact command strings the Jenkinsfile will use (decided once, reused verbatim — not approximated now and "cleaned up" later), then only Generate the Jenkinsfile. Auto-fix now sits inside the preflight loop (fix, rerun, repeat) rather than being deferred entirely to another skill. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… move it pre-commit Jenkins is not permitted to commit changes back to the repository, so a CI-side Auto-fix code style stage has no useful outcome: either its rewrites are discarded when the container is removed, or it has to auto-commit — the pattern EEA policy avoids. Remove the stage from the Required workflow, Mandatory pipeline contract, Stage blueprint, Container cleanup rules, and the canonical eea-jenkinsfile-template.md, replacing it with a new "Pre-commit auto-fix, not a Jenkins stage" section: auto-fixing happens before the commit (already Phase 3 of the Required workflow), using the same Dockerfile.test image and command strings the Jenkins `Code linting` stage verifies with --check/no-fix flags. Same change in code-quality's SKILL.md, EEA-OVERRIDES.md, and references/jenkins-quality-gates.md (which previously documented an "Auto-commit option" for CI — removed, it directly contradicted EEA policy). Added an explicit operating principle: auto-fix using the same tools/image Jenkins will verify with, every time code is added or modified, before it's committed — not something to defer to a CI stage. Left the two verbatim historical example Jenkinsfiles (python-egg-plone, nodejs-volto) unchanged as real production records, but added a note that their CI-side auto-commit pattern is not the currently recommended approach. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…r for routine pre-commit auto-fix Building/running Dockerfile.test on every single commit is real overhead a developer shouldn't pay for routine changes. Add a version-check procedure to code-quality: compare the tool versions Dockerfile.test pins against what's installed on the host (prefer the project-local install — .venv, node_modules — over global). When they match, auto-fix runs natively (fast, no docker build/run); when they don't match or a tool is missing locally, fall back to Docker for that tool rather than silently trusting a mismatched version. This doesn't weaken the existing requirement: building and validating Dockerfile.test against the exact Jenkinsfile commands is still mandatory the first time the Jenkinsfile is created (jenkins-pipeline's Required workflow Phase 2/3) — that's what proves the commands actually work in Jenkins' environment. The native-tools shortcut only applies afterward, for ongoing day-to-day auto-fixing once that parity is already confirmed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…an't, add failed-build diagnosis reference code-quality: add "Nothing known-broken gets committed" — running checks and auto-fixing before every commit isn't optional, and neither is fixing whatever auto-fix can't reach (real bugs, type errors, non-mechanical lint, failing tests). Format/lint auto-fix is fast enough to run every commit; tests take longer but that's a reason to budget time for them, not skip them — run what's relevant to the change every commit, the full suite before push/PR. Never commit red with a "fix it in CI" plan, since Jenkins can't commit a fix back regardless. jenkins-pipeline: add references/diagnosing-failed-builds.md. EEA Jenkins publishes build results as GitHub Checks (github-checks plugin, confirmed via real build logs all session) — this is not the same as Jenkins' GitHub OAuth login, it's the other direction: Jenkins reports to GitHub, and `gh` reads that, no Jenkins credentials needed. Workflow: check status via `gh pr checks`/`gh api .../check-runs` before asking the user for a console log, map the failing check name to its Jenkinsfile stage, reproduce that stage's exact command locally (native or Docker per code-quality's native-vs-Docker guidance), fix and verify locally before pushing again. Note: `gh` wasn't available to verify this in the current sandbox — the guidance is grounded in the confirmed GitHub Checks plugin/log behavior, with an explicit fallback to asking the user directly when `gh` isn't set up. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…PI token Extend diagnosing-failed-builds.md with a section for when GitHub Check status/summary isn't enough and a developer wants the agent reading the actual Jenkins console log. Requires a personal API token generated at https://ci.eionet.europa.eu/user/<github-username>/security/ (the same username used for Jenkins' GitHub OAuth login) — clarified this is a one-time, per-developer credential passed via environment variables, never committed anywhere. Includes the exact curl calls for console log and build metadata, the nested-folder job path shape, and build selectors (lastBuild/lastFailedBuild/lastSuccessfulBuild). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…efault branch is analyzed Reported bug turned out not to be a badge URL bug: an unscoped badge (no &branch=) shows the project's designated default branch specifically, not "whatever was last analyzed." If every Jenkins run so far only analyzed a feature branch or a PR, the default branch (main) has no data yet and the badge renders "Quality gate has not been found" / "Measure has not been found" — confirmed directly by curling both endpoints with and without &branch=: without it, not found; with the branch that was actually analyzed, real data (passed gate, 57.9% coverage). Resolves itself once Jenkins runs on the default branch directly (typically right after the Jenkinsfile PR merges). Document this so it's told to the developer upfront during initial badge setup, not mistaken for a bug later. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ult branch Covers the case where a developer explicitly wants a badge for a specific long-lived branch (develop, a release line) rather than the default branch, matching the older EEA dual-row README convention (one row per branch). Gives the exact &branch= URL pattern for both SonarQube badge types and notes: this branch must have actually been analyzed with sonar.branch.name=<branch> at least once (same "not found" failure mode as an unanalyzed default branch), and it should be reserved for long-lived branches — a short-lived branch's Sonar data may eventually be cleaned up, at which point the badge goes stale with no code change to explain it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… fit a 4-option UI cap Confirmed real occurrence: a from-zero test session's multi-select question bundled Reliability/Security/Maintainability into one "ratings" option and dropped Duplications and Security Hotspots Reviewed entirely — because the question tool used caps out at 4 selectable options, and 7 badges don't fit. The developer never saw those as individual choices. Require all 7 badges stay individually choosable regardless of a tool's option limit: split into two batched multi-select questions, or enumerate the complete list as plain text in the question itself so every name is visible even if not all are clickable. Bundling multiple distinct metrics under one label to fit a limit is exactly the failure mode to avoid. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ng more Detect existing Jenkins/SonarQube badges in the README before asking anything else. If found, tell the developer what's there and ask whether to keep, replace, or leave untouched, rather than defaulting to appending new badges alongside old ones (duplicate/redundant rows nobody wanted). If the developer's request already says regenerate/replace/redo, that's answer enough — remove the existing recognized badges first, then add the newly-chosen set, no need to ask again. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ayout guidance Four fixes from real usage: 1. security_hotspots_reviewed is not a valid metric for the badge API — confirmed via the server's own HTTP 400 response listing accepted values. Corrected to security_hotspots (a count), the actual accepted key, verified working with a live curl. 2. The branch question now proactively warns when the current branch is short-lived and will be merged/deleted: badges pinned to it go stale or break once it's gone, so default to the repository's default branch unless the developer explicitly wants a non-default one too. 3. Documented that the Quality Gate badge renders as a large square icon (~94x71px) vs. every other badge here being a small flat line (~20px) — don't interleave it into the flat-badge row. Put it on its own line, then Pipeline + all measure badges together on one shared line. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…art release patterns Both from real EEA Jenkinsfile examples, documented as optional patterns (most repos don't need them) rather than added to the default stage blueprint: - Multi-arch build/release: classic docker.build()/dockerImage.push() Groovy DSL can't produce multi-platform images at all — buildx is required. Documents the one-time QEMU/binfmt + persistent builder setup, a "Multi-platform build check" stage (builds for every platform with no -t/--push/--load, since buildx can't --load a multi-platform result locally — pure sanity check, real cost/coverage tradeoff on when to run it), and the actual release stage (buildx build --push, since --push is the substitute for --load here, plus a node-label note since QEMU-emulated arm64 is meaningfully heavier than native). - Helm chart / Rancher catalog release stage: the same eeacms/gitflow tool used everywhere else, gated on buildingTag(). Notably safe to add even before the repository has a chart yet — a no-op until one exists at the configured path, so adding it preemptively avoids a future Jenkinsfile change when a chart eventually gets added. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ockerhub' -> 'jekinsdockerhub') Confirmed by a real build failure: "Could not find credentials entry with ID 'dockerhub'" — a from-zero generated Jenkinsfile inherited this ID from the skill's own canonical eea-jenkinsfile-template.md, which had never been validated (mission-aipossible's original Jenkinsfile also used it, but its Release stage's `when` gate meant it never actually ran until this test). 'jekinsdockerhub' is confirmed working across multiple independent real EEA Jenkinsfiles (eea.docker.jenkins.master, eionet.xmlconv) that actually exercise this stage in production. Fixed in both the canonical template's single-arch release stage and the multi-arch release stage added earlier. Also fixed the Helm chart section's explanatory note, which had cited the now-corrected 'dockerhub' value as an example of ID naming variance across repos. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… a version file Real bug hit in production testing: a repo tagged 0.1.0 pushed a Docker image tagged 0.2.0, because the canonical template's Versioning stage read the version from package.json/pyproject.toml unconditionally — completely ignoring env.TAG_NAME, the actual git tag that triggered the build. The two sources can silently drift apart. Fix: on a tag build, use env.TAG_NAME directly as the image version (guarantees the Docker tag always matches the git tag), and fail the build loudly if the version file disagrees, catching a forgotten version bump at build time instead of silently shipping a mismatched image. Applied to both the canonical eea-jenkinsfile-template.md and documented as an explicit rule in "Versioning and release guidance" (previously just said "derive from the source of truth... package.json, pyproject.toml, Git tags, etc." without specifying which one wins when they disagree). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e; add reverse trigger Multi-arch builds and the Helm chart/Rancher Fleet release stage are now explicitly things to ask the developer about when finalizing a Jenkinsfile, not something the agent silently decides either way (mirrors the existing badge-asking pattern). Both framed as addable later at no cost, so the developer isn't pressured into a decision before they have the information for it. Added the reverse trigger: if a developer asks to add a Helm chart to a repo that already has a working Jenkinsfile, propose adding the release stage now that there's an actual chart to release, rather than leaving automation as a separate thing to remember. Documented that the Helm chart release stage also updates Rancher Fleet files when they exist — Fleet is EEA's GitOps continuous-delivery mechanism for Rancher 2-managed Kubernetes, kept in sync by the same eeacms/gitflow call, no separate stage needed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ng, default to amd64 Don't assume the developer already knows what CPU architectures mean when asking about multi-arch builds. Give a plain-language explanation (amd64 = what almost all servers/dev machines use, the default; arm64 = Apple Silicon and some ARM cloud/edge hosts) and state explicitly that amd64-only is the default when nothing is said. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…real Java/Maven repo Ran the skill unmodified against a fresh Java/Maven test repo via a context-isolated agent (no knowledge of this session's prior fixes) to see where its own instructions, not prior tribal knowledge, left it guessing: - GIT_NAME = env.JOB_BASE_NAME in the generic template resolves to the branch name on a Multibranch Pipeline job, not the repo name — silently wrong on the exact branch the Docker Hub release stage is gated on. Confirmed as a real latent bug by cross-checking eea-ai-mission-aipossible's actual working Jenkinsfile, which hardcodes its image basenames instead. Fixed to a hardcoded literal, matching both that and the real Java example. - Phase 2 of the Required workflow unconditionally mandated creating Dockerfile.test before Phase 4 could point at a references/examples/ pattern (Java/Maven) that doesn't use one at all. Added an explicit Phase 1 discovery step and Phase 2 branch so tool-based pipelines skip Dockerfile.test entirely instead of generating an unused one. - The Trivy severity gate section led with the complex .trivyignore docker create/cp/start example and buried the "no .trivyignore? use plain docker run --rm" guidance in a trailing sentence — easy to copy the wrong pattern. Reordered so the common case (no .trivyignore) leads. - The Mandatory pipeline contract's Integration test requirement had no escape hatch for repos with nothing to integration-test against (a CLI tool with no server/DB) — added one, scoped narrowly to that case.
Confirmed live against a real PR (eea/eea-ai-mission-aipossible#33): the aggregate Jenkins check's output.text carries a full per-stage timing breakdown, but other checks (a GitHub Actions dependency scan, in this case) report output.summary/text as null with a nonzero annotations_count instead — the actual finding is only reachable via the separate check-runs/<id>/annotations endpoint, which the doc didn't mention.
Both caught by watching eea-ai-java-test-app's real Jenkins builds after generating a Jenkinsfile from this skill: - A version-numbered Docker tag was pushed on every default-branch build, not just on an actual git tag/release — a repo whose version file says "0.1.0" would push that tag on the very next merge, before 0.1.0 was ever tagged. Now the version tag only pushes on buildingTag(); :latest tracks the newest default-branch build instead. Also fixes the Release-on-Docker-Hub `when` clause to actually include buildingTag(), since without it a tag build never reached the stage that was supposed to push its version tag in the first place. - Every branch check meaning "the default branch" hardcoded 'main' (or 'master' in the generic template) instead of checking the repo's actual default branch. Added a Phase 1 discovery step and a DEFAULT_BRANCH literal (same pattern as GIT_NAME) used everywhere instead. Verified both fixes against the two live test repos: pushed to eea-ai-java-test-app's Jenkinsfile (main) and confirmed both real, non-EEA-AI-org-default default branches are actually 'main' via the GitHub API rather than assumed.
…r to a warning Per user feedback: a git tag build should always push that tag as the Docker image version, unconditionally — the version file is a useful cross-check, not a gate. Hard-failing on a mismatch blocks legitimate cases: alpha/beta/rc pre-releases, a v-prefixed tag convention (v1.2.3 vs a version file saying 1.2.3), or a version file intentionally not bumped until a separate release-automation step runs. Now mismatches log a warning and mark the build UNSTABLE instead of failing outright, and a leading 'v' is stripped before comparing to cut false-positive noise from that specific convention.
demarant
reviewed
Aug 7, 2026
demarant
left a comment
demarant
left a comment
Member
There was a problem hiding this comment.
Code Review
Overall: solid PR, blocked on a single corrupted file + 1 cross-skill contradiction.
Blocker
catalog.yaml: every line is committed with a literal NNNN| line-number prefix (looks like cat -n output). yaml.safe_load fails at line 10, column 5 with expected '<document start>', but found '<block mapping start>'. This is the validate-harness CI failure (job Validate catalog.yaml step). Re-export the file without the prefix and re-commit. Not a structural issue — just a corrupted commit.
Cross-skill contradiction
src/skills/testing/SKILL.md:96-99 and references/docker-test-parity.md:23 recommend docker run --rm -v "$PWD:/workspace" -w /workspace myapp-test pytest tests as the developer-reproducible flow, but src/skills/jenkins-pipeline/EEA-OVERRIDES.md:113-156 (the Docker-outside-of-Docker constraint) explicitly states that bind-mounting $PWD/$WORKSPACE into a spawned container silently produces an empty directory on EEA docker-host agents (the agent's $WORKSPACE is not a real path on the dockerd's host). The "source is already in the test image via COPY . ." rule the jenkins-pipeline skill was written to enforce is the EEA-correct approach. The bind-mount example works on a developer's local box but breaks the moment it gets copy-pasted into the Jenkinsfile the testing skill is meant to keep aligned with. Align the two skills (or cross-reference the constraint from testing/SKILL.md).
EEA-01 hygiene
src/skills/jenkins-pipeline/SKILL.md:519-521 and :637 hardcode https://sonarqube.eea.europa.eu/api/project_badges/... and https://ci.eionet.europa.eu/buildStatus/icon?job=... — EEA-specific internal endpoints. Per harness/EEA-HARNESS.md (EEA-01), EEA-specific values belong in EEA-OVERRIDES.md, not the base SKILL.md. Move them, or document the rationale for keeping them in the base.
Metadata contradiction
src/skills/jenkins-pipeline/metadata.json:11 declares upstream: aj-geddes/useful-ai-prompts while eeaspecific: true and the SKILL.md is fully rewritten with EEA-specific content. Pick one: either set upstream: null and treat it as an EEA-original skill, or mark eeaspecific: false and keep the upstream as the authoritative source. Same issue, less severe, in code-quality/SKILL.md:11 and quality-fixes/SKILL.md:11 (where eeaspecific: true is set without an upstream_source at all).
Nice-to-haves
src/skills/testing/EEA-OVERRIDES.md:20-22(wrappers clause) softens the EEA-01 "developer reproducibility" rule into "allowed if careful." Either tighten or drop the testing skill — its 109 lines + 26-line reference largely duplicate jenkins-pipeline's Required workflow + code-quality's Jenkins compatibility section + docker-expert'sDockerfile.testcontract.src/skills/jenkins-pipeline/references/dockerfile-test-template.md:36: add a one-liner that the test image is shared between dev and CI to prevent drift.src/skills/jenkins-pipeline/EEA-OVERRIDES.md:377-378: three credential-derived env vars (GIT_TOKEN,DOCKERHUB_USER,DOCKERHUB_PASS) passed in onedocker run -eline — ifeeacms/gitflowever logs its env on error, all three land on the same log line.CHANGELOG.md:7-99: five separate2026-07-29entries for one PR. Consider one combined entry with sub-bullets.
Strengths
- 31 well-scoped commits, conventional-commits format, each with a clear "what + why" body.
jenkins-pipeline/EEA-OVERRIDES.mdis the strongest part: Docker-outside-of-Docker constraint, theansicolorfailure note, the multi-archbuildxpattern, thejekinsdockerhubvsdockerhubcorrection, the GitHubEEAalm_settingsbinding — all real, on-the-ground lessons captured in prose.references/diagnosing-failed-builds.mdis genuinely good:gh pr checks→check-runs→ annotations → console log via personal token, with an explicit "don't reach for crumb handling" note.- Secrets scan: clean. All
api_key|token|secret|passwordmatches in the diff are variable names, credential IDs (jekinsdockerhub,eea-jenkins-token), or conceptual references — zero literal credentials. - CI:
check-changelogpassing,build-syncpassing,lintpassing,upstream-syncskipping (expected for EEA-original skills).
…kill, fix 2 real bugs from live re-test PR #1 review fixes: - catalog.yaml was committed with a literal cat -n style line-number prefix on every line; stripped, now parses as valid YAML (CI blocker). - testing/SKILL.md and docker-test-parity.md recommended bind-mounting $PWD into a test container, which silently produces an empty directory on EEA Jenkins docker-host agents; both now rely on COPY . . baked into the image instead, matching the Docker-outside-of-Docker constraint jenkins-pipeline already documents. - jenkins-pipeline's upstream claimed aj-geddes/useful-ai-prompts while eeaspecific: true and fully rewritten; set to null, reworded as EEA-original, consistent with code-quality/quality-fixes. - catalog.yaml was missing an entry for the testing skill entirely. Refocus testing skill: - De-duplicated the Dockerfile.test artifact contract, previously stated near-verbatim in three places; docker-expert/EEA-OVERRIDES.md is now the single canonical source, others cross-reference it. - Rewrote testing/SKILL.md around its actual purpose: parity-command guidance trimmed to a short contract, and two new sections ("Test quality over test presence", "Closing a coverage gap") make real coverage improvement -- tests that exercise actual behavior, not ones that just move the number -- a first-class part of the skill. Deleted the now-fully-redundant docker-test-parity.md reference. - code-quality/SKILL.md cross-references testing instead of restating the same-Docker-image/same-command-strings principle. Fixes from live re-testing against a real Java/Maven repo (two fresh branches, two context-isolated agents, real Jenkins runs): - pom.xml never declared org.sonarsource.scanner.maven:sonar-maven-plugin, so `mvn sonar:sonar`'s bare-prefix resolution depended on whichever Jenkins node ran the build having that plugin group in its own ambient settings.xml -- confirmed via two otherwise-identical builds where one passed and the next failed with "No plugin found for prefix 'sonar'". - A generated Jenkinsfile pre-declared RELEASE_IMAGE = '' in the top-level environment{} block, then tried to override it via env.RELEASE_IMAGE = "..." in a script{} step -- the pre-declared value silently wins for every sh step. Confirmed via a real Jenkins failure: `docker build -t '' .` -> invalid tag "". EEA-OVERRIDES.md now has a dedicated section on this general Declarative Pipeline pitfall. - Documented EEA's org-wide GitHub<->Jenkins auto-trigger integration in diagnosing-failed-builds.md, so a missing build right after a push reads as "not built yet", not "something is broken". - Added a not-silently-inventing-a-release-Dockerfile carve-out to the mandatory pipeline contract, and made the GitHubEEA SonarQube binding curl call surface failures instead of failing silently. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Author
Addressed review feedback + live re-testing fixesPushed Review findings (all addressed)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #302663