You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Drafted at the request of Gustavo Betances, following up from the AI Agent Vertex CPT PR work (#8226).
Problem
.github/workflows/MERGE_QUEUE_HELM_TEST.yaml only triggers on:
on:
push:
branches:
- mainworkflow_dispatch:
merge_group:
There's no pull_request trigger at all. That means the run-ai-agent-cpt job ("AI Agent E2E Tests (CPT)"), which runs the real-LLM CPT suite for the agentic-ai module, gives a dev working on a PR zero automatic signal until the PR reaches the merge queue or actually lands on main. Today the job:
and depends on build-image (which builds and pushes the connectors-bundle / connectors-bundle-saas Docker images that the CPT suite pulls at runtime via CONNECTORS_IMAGE_NAME/CONNECTORS_IMAGE_VERSION).
So today, if you touch connectors/agentic-ai/** or connectors-e2e-test/connectors-e2e-test-agentic-ai/** in a PR, the only way to get this suite to run before merge queue is to manually workflow_dispatch the entire Helm-test workflow — which also builds/publishes the Docker image and kicks off unrelated jobs (trigger-sm-e2e, trigger-saas-e2e), not just the AI Agent CPT job.
This module has two real-LLM provider suites needing this treatment: the original OpenAI suite (AiAgentE2ETestIT, added in #6940, on main) and the new Google Vertex AI suite (GoogleVertexAiE2ETestIT, added in #8226, still open/unmerged as of this update). Both benefit equally from an earlier feedback loop.
Decision: dedicated standalone workflow, not a piggyback on MERGE_QUEUE_HELM_TEST.yaml
Add a new workflow (e.g. AI_AGENT_CPT_PR.yml), modeled directly on the existing BUILD_PR_DOCKER_IMAGES.yml pattern — that file is the closest real precedent in this repo for "label-gated PR-scoped job that needs secrets and a Docker image."
Do not add pull_request to MERGE_QUEUE_HELM_TEST.yaml itself: trigger-sm-e2e and trigger-saas-e2e both gate only on needs.build-image.outputs.connectors-version != '', which is populated on every triggering event — so reusing that chain would also arm two unrelated e2e-trigger jobs on every labeled PR unless retrofitted with additional event guards across three jobs.
Structure of the new workflow: one job, no cross-job image handoff — build bundle/default-bundle only (linux/amd64 only, no multi-arch) and load it locally into the runner's Docker daemon (load: true, not push: true), then run the CPT suite in that same job. Match the runner this module already runs on in nightly/branch e2e (gcp-core-8-default) rather than run-ai-agent-cpt's current ubuntu-latest.
Resolved design decisions
(a) Dedicated workflow vs. piggybacking — Dedicated (see above). Piggybacking on MERGE_QUEUE_HELM_TEST.yaml pulls in unrelated jobs and requires retrofitting event guards across the reactor.
plus ref: ${{ github.event.pull_request.head.sha }} on checkout. pull_request_target would hand a Vault AppRole and a real OPENAI_API_KEY/Vertex service-account to unvetted fork code before it's reviewed — a materially different risk than this repo's existing pull_request_target usages (ENFORCE_QA_APPROVAL.yml, BACKPORT_PR.yml), which only read PR metadata (title, labels, review state) and never execute fork-authored code. Fork PRs get no automatic real-LLM CPT signal under this design — same accepted tradeoff BUILD_PR_DOCKER_IMAGES.yml already makes.
⚠️Do not copy NIGHTLY_E2E.yml's fork gate — it has a live operator-precedence bug (independently re-verified against origin/main):
github.event_name != 'pull_request' ||
(github.event.pull_request.head.repo.fork == false && (action == 'labeled' && label == 'e2e-tests')) ||
(action == 'synchronize' && contains(labels, 'e2e-tests')) ← no fork check on this branch
&& binds tighter than ||, so the fork check only guards the labeled branch; a labeled fork PR passes the gate on every subsequent synchronize push with full secrets access. BUILD_PR_DOCKER_IMAGES.yml's gate (head.repo.full_name == github.repository, correctly parenthesized) does not have this flaw — copy that one instead. The NIGHTLY_E2E.yml bug is a distinct, pre-existing issue worth its own separate ticket.
(c) Re-trigger semantics — types: [labeled, synchronize], matching both existing label-triggered precedents in this repo. Iterative LLM-prompt/tool-calling work is exactly the case where re-running on every push matters most. dorny/paths-filter must be the first real step after checkout, and every subsequent step (build, docker login/build, test) must carry if: steps.changes.outputs.ai == 'true' — since this workflow merges the build and test into one job (unlike run-ai-agent-cpt, which gets its image for free from a separate needs: build-image job), a labeled PR touching no agentic-ai files must not pay for a bundle build at all, not just skip the test step.
(d) Cost guardrails — Concurrency group scoped to the PR number only, deliberately without the label-name key that BUILD_PR_DOCKER_IMAGES.yml uses:
Omitting the label from the key means removing-then-re-adding the label cancels any in-flight run instead of queuing a duplicate — the actual guard against label-flapping cost abuse. Set timeout-minutes: 30, though note real measured timings on main (build-image 8m21s + AI Agent E2E Tests (CPT) 14m23s ≈ 23min serialized) leave only ~7min slack once Docker build + reactor overhead is added in a single job — may need raising after a first real run.
Skip -Dfailsafe.rerunFailingTestsCount=0 — verified it's already the default (zero references to it anywhere in this repo), so it guards nothing. The actual real-money cost amplifier is inside the BPMN itself: ai-agent-e2e-openai.bpmn sets retries="3" on the AI Agent job worker and maxModelCalls=20 — one flaky agent job can retry 3× within a single test, each retry up to 20 real model calls. Any cost guardrail belongs at that level (or a hard per-test timeout), not at the failsafe-retry flag.
(e) Label choice — New dedicated label (e.g. ai-agent-cpt, does not exist yet — creating it is a prerequisite step, and it needs a description spelling out the cost so it isn't applied casually), not the existing agentic-ai label. agentic-ai already has a machine consumer: ADD_TO_PROJECT.yml uses it to route issues/PRs onto the agentic-ai project board, and it's applied broadly (100+ PRs historically, vs. 36 for docker-images-required and 63 for e2e-tests) — often by routine triage, not by someone deliberately requesting a CI run. Reusing it would mean ordinary board-triage labeling silently burns real LLM API quota. Both real trigger-label precedents in this repo (docker-images-required, e2e-tests) are purpose-built, never a general categorization label — follow that pattern here.
Scope — covers both real-LLM suites (AiAgentE2ETestIT for OpenAI, GoogleVertexAiE2ETestIT for Vertex once #8226 merges) since both already run out of the same -Pit-real-llm Maven profile / module. Note the judge model itself (application-it-real-llm.yml) is hardcoded to provider: openai/gpt-5-nano — so a "Vertex-only" run still requires OPENAI_API_KEY and burns OpenAI quota per assertion, not just Vertex quota. Both secrets are mandatory regardless of which suite triggered the run.
Gaps found in review (self-review + advisor pass), fixed above or flagged as open
A second pass against the actual repo files and GitHub Actions security-hardening guidance surfaced issues in the first draft of this plan, corrected inline above where possible:
Missing Minimus registry login.bundle/default-bundle's Dockerfile is FROM reg.mini.dev/1212/openjre-base:... — building it needs a docker/login-action against reg.mini.dev with the REGISTRY_MINIMUS_PSW Vault secret (see MERGE_QUEUE_HELM_TEST.yaml), not just Harbor. The plan above now avoids the issue a different way: use load: true instead of pushing anywhere, but the base-image pull during docker build still needs the Minimus login/secret regardless of push target.
Push-to-registry was unnecessary. Since build and test now happen in the same job, the image never needs to leave the runner — load: true (no push:) makes it available to testcontainers locally, which also removes Harbor credentials from this workflow's blast radius and avoids orphan -run<id>-a<n> tags accumulating in Harbor with no GC story. Reflected in the Decision section above.
e2e-runner-overrides.json is not actually read by any workflow — verified E2E_BRANCH_RUN.yml inlines the same JSON as a hardcoded literal rather than opening the file; the JSON itself has no live consumer. Removed the citation of it as authoritative; the new workflow should just hardcode runs-on: gcp-core-8-default directly with a comment explaining why.
Element-templates-cli pinning — BUILD_PR_DOCKER_IMAGES.yml installs it unpinned (npm install --global element-templates-cli), but run-ai-agent-cpt (the job actually being replicated) pins it via .github/workflows/package.json. Copy the pinned form from run-ai-agent-cpt, not the unpinned one from BUILD_PR_DOCKER_IMAGES.yml.
-pl ... -am reactor double-build risk — building bundle/default-bundle with --also-make and then separately running verify -pl connectors-e2e-test-agentic-ai -am in the same job risks building parts of the reactor twice. Worth using install -DskipTests -DskipChecks once for the modules both steps need, then a plain verify -pl <module> without -am for the test step, mirroring what E2E_BRANCH_RUN.yml already does deliberately for the same reason.
Structured-secret leak risk with the Vertex service-account JSON (once test(agentic-ai): add Google Vertex AI real-LLM coverage and parameterise the E2E suite over providers #8226 lands). That PR forwards the GCP service-account JSON (multi-line, with an embedded PEM key) into the connectors container's env via camunda.process-test.connectors-secrets. GitHub's log masking is exact-string match only — sub-fields of a JSON blob (private_key_id, client_email, PEM body lines) aren't individually registered as secrets, so a container-log dump on test failure could leak them unmasked. This predates and is independent of the new PR-trigger workflow itself, but is a real gap in how test(agentic-ai): add Google Vertex AI real-LLM coverage and parameterise the E2E suite over providers #8226 already handles that credential in run-ai-agent-cpt today. Consider decoding to a file with restricted permissions instead of an env var, or GCP Workload Identity Federation via OIDC to drop the long-lived key entirely — flagging for separate follow-up, out of scope for this ticket.
No failure-feedback path decided.run-ai-agent-cpt currently ends with a step calling ./.github/actions/observe-build-status — that action directory does not exist in this repo (.github/actions/ only contains codeowners-slack-mentions and post-failure-feedback), confirmed via a live completed run's job log showing ##[error]Can't find 'action.yml'... — masked because the step has continue-on-error: true. This is a separate, pre-existing bug in run-ai-agent-cpt/MERGE_QUEUE_HELM_TEST.yaml, worth its own ticket, and NOT something the new PR-triggered workflow should copy. post-failure-feedback is the working sibling action already used elsewhere in the same file — if failure feedback on the PR is wanted, use that pattern instead.
Must not become a required status check. A [labeled, synchronize]-triggered workflow produces no check run at all on unlabeled PRs — making it a required check would block every unlabeled PR forever. This is a manual branch-protection setting, called out here so it isn't set by accident.
permissions: block omitted from the plan. Add job-level contents: read (matching BUILD_PR_DOCKER_IMAGES.yml); add pull-requests: write only if a PR-comment feedback step is added per point 7.
Confirmed correct on review: default-bundle-only is right (the SaaS bundle is genuinely unused by the real-LLM CPT suite — application-it-real-llm.yml only references connectors-bundle), linux/amd64-only is correct (the runner is amd64), and the NIGHTLY_E2E.yml fork-gate bug is real exactly as described in point (b) above.
Still open
Exact timeout-minutes value once a first real run's timing is measured in the new single-job structure (see point (d) above).
Whether to add a PR-comment failure-feedback step (via post-failure-feedback) or leave failures visible only via the check run itself.
Follow-up ticket for the NIGHTLY_E2E.yml fork-gate precedence bug (independent of this ticket).
Follow-up ticket for the broken ./.github/actions/observe-build-status reference in MERGE_QUEUE_HELM_TEST.yaml (independent of this ticket).
Drafted at the request of Gustavo Betances, following up from the AI Agent Vertex CPT PR work (#8226).
Problem
.github/workflows/MERGE_QUEUE_HELM_TEST.yamlonly triggers on:There's no
pull_requesttrigger at all. That means therun-ai-agent-cptjob ("AI Agent E2E Tests (CPT)"), which runs the real-LLM CPT suite for the agentic-ai module, gives a dev working on a PR zero automatic signal until the PR reaches the merge queue or actually lands onmain. Today the job:and depends on
build-image(which builds and pushes theconnectors-bundle/connectors-bundle-saasDocker images that the CPT suite pulls at runtime viaCONNECTORS_IMAGE_NAME/CONNECTORS_IMAGE_VERSION).So today, if you touch
connectors/agentic-ai/**orconnectors-e2e-test/connectors-e2e-test-agentic-ai/**in a PR, the only way to get this suite to run before merge queue is to manuallyworkflow_dispatchthe entire Helm-test workflow — which also builds/publishes the Docker image and kicks off unrelated jobs (trigger-sm-e2e,trigger-saas-e2e), not just the AI Agent CPT job.This module has two real-LLM provider suites needing this treatment: the original OpenAI suite (
AiAgentE2ETestIT, added in #6940, onmain) and the new Google Vertex AI suite (GoogleVertexAiE2ETestIT, added in #8226, still open/unmerged as of this update). Both benefit equally from an earlier feedback loop.Decision: dedicated standalone workflow, not a piggyback on
MERGE_QUEUE_HELM_TEST.yamlAdd a new workflow (e.g.
AI_AGENT_CPT_PR.yml), modeled directly on the existingBUILD_PR_DOCKER_IMAGES.ymlpattern — that file is the closest real precedent in this repo for "label-gated PR-scoped job that needs secrets and a Docker image."Do not add
pull_requesttoMERGE_QUEUE_HELM_TEST.yamlitself:trigger-sm-e2eandtrigger-saas-e2eboth gate only onneeds.build-image.outputs.connectors-version != '', which is populated on every triggering event — so reusing that chain would also arm two unrelated e2e-trigger jobs on every labeled PR unless retrofitted with additional event guards across three jobs.Structure of the new workflow: one job, no cross-job image handoff — build
bundle/default-bundleonly (linux/amd64only, no multi-arch) and load it locally into the runner's Docker daemon (load: true, notpush: true), then run the CPT suite in that same job. Match the runner this module already runs on in nightly/branch e2e (gcp-core-8-default) rather thanrun-ai-agent-cpt's currentubuntu-latest.Resolved design decisions
(a) Dedicated workflow vs. piggybacking — Dedicated (see above). Piggybacking on
MERGE_QUEUE_HELM_TEST.yamlpulls in unrelated jobs and requires retrofitting event guards across the reactor.(b) Fork-PR / secrets safety —
pull_request, neverpull_request_target. CopyBUILD_PR_DOCKER_IMAGES.yml's fork gate verbatim:plus
ref: ${{ github.event.pull_request.head.sha }}on checkout.pull_request_targetwould hand a Vault AppRole and a realOPENAI_API_KEY/Vertex service-account to unvetted fork code before it's reviewed — a materially different risk than this repo's existingpull_request_targetusages (ENFORCE_QA_APPROVAL.yml,BACKPORT_PR.yml), which only read PR metadata (title, labels, review state) and never execute fork-authored code. Fork PRs get no automatic real-LLM CPT signal under this design — same accepted tradeoffBUILD_PR_DOCKER_IMAGES.ymlalready makes.NIGHTLY_E2E.yml's fork gate — it has a live operator-precedence bug (independently re-verified againstorigin/main):&&binds tighter than||, so the fork check only guards thelabeledbranch; a labeled fork PR passes the gate on every subsequentsynchronizepush with full secrets access.BUILD_PR_DOCKER_IMAGES.yml's gate (head.repo.full_name == github.repository, correctly parenthesized) does not have this flaw — copy that one instead. TheNIGHTLY_E2E.ymlbug is a distinct, pre-existing issue worth its own separate ticket.(c) Re-trigger semantics —
types: [labeled, synchronize], matching both existing label-triggered precedents in this repo. Iterative LLM-prompt/tool-calling work is exactly the case where re-running on every push matters most.dorny/paths-filtermust be the first real step after checkout, and every subsequent step (build, docker login/build, test) must carryif: steps.changes.outputs.ai == 'true'— since this workflow merges the build and test into one job (unlikerun-ai-agent-cpt, which gets its image for free from a separateneeds: build-imagejob), a labeled PR touching no agentic-ai files must not pay for a bundle build at all, not just skip the test step.(d) Cost guardrails — Concurrency group scoped to the PR number only, deliberately without the label-name key that
BUILD_PR_DOCKER_IMAGES.ymluses:Omitting the label from the key means removing-then-re-adding the label cancels any in-flight run instead of queuing a duplicate — the actual guard against label-flapping cost abuse. Set
timeout-minutes: 30, though note real measured timings onmain(build-image8m21s +AI Agent E2E Tests (CPT)14m23s ≈ 23min serialized) leave only ~7min slack once Docker build + reactor overhead is added in a single job — may need raising after a first real run.Skip
-Dfailsafe.rerunFailingTestsCount=0— verified it's already the default (zero references to it anywhere in this repo), so it guards nothing. The actual real-money cost amplifier is inside the BPMN itself:ai-agent-e2e-openai.bpmnsetsretries="3"on the AI Agent job worker andmaxModelCalls=20— one flaky agent job can retry 3× within a single test, each retry up to 20 real model calls. Any cost guardrail belongs at that level (or a hard per-test timeout), not at the failsafe-retry flag.(e) Label choice — New dedicated label (e.g.
ai-agent-cpt, does not exist yet — creating it is a prerequisite step, and it needs a description spelling out the cost so it isn't applied casually), not the existingagentic-ailabel.agentic-aialready has a machine consumer:ADD_TO_PROJECT.ymluses it to route issues/PRs onto the agentic-ai project board, and it's applied broadly (100+ PRs historically, vs. 36 fordocker-images-requiredand 63 fore2e-tests) — often by routine triage, not by someone deliberately requesting a CI run. Reusing it would mean ordinary board-triage labeling silently burns real LLM API quota. Both real trigger-label precedents in this repo (docker-images-required,e2e-tests) are purpose-built, never a general categorization label — follow that pattern here.Scope — covers both real-LLM suites (
AiAgentE2ETestITfor OpenAI,GoogleVertexAiE2ETestITfor Vertex once #8226 merges) since both already run out of the same-Pit-real-llmMaven profile / module. Note the judge model itself (application-it-real-llm.yml) is hardcoded toprovider: openai/gpt-5-nano— so a "Vertex-only" run still requiresOPENAI_API_KEYand burns OpenAI quota per assertion, not just Vertex quota. Both secrets are mandatory regardless of which suite triggered the run.Gaps found in review (self-review + advisor pass), fixed above or flagged as open
A second pass against the actual repo files and GitHub Actions security-hardening guidance surfaced issues in the first draft of this plan, corrected inline above where possible:
bundle/default-bundle'sDockerfileisFROM reg.mini.dev/1212/openjre-base:...— building it needs adocker/login-actionagainstreg.mini.devwith theREGISTRY_MINIMUS_PSWVault secret (seeMERGE_QUEUE_HELM_TEST.yaml), not just Harbor. The plan above now avoids the issue a different way: useload: trueinstead of pushing anywhere, but the base-image pull duringdocker buildstill needs the Minimus login/secret regardless of push target.load: true(nopush:) makes it available to testcontainers locally, which also removes Harbor credentials from this workflow's blast radius and avoids orphan-run<id>-a<n>tags accumulating in Harbor with no GC story. Reflected in the Decision section above.e2e-runner-overrides.jsonis not actually read by any workflow — verifiedE2E_BRANCH_RUN.ymlinlines the same JSON as a hardcoded literal rather than opening the file; the JSON itself has no live consumer. Removed the citation of it as authoritative; the new workflow should just hardcoderuns-on: gcp-core-8-defaultdirectly with a comment explaining why.BUILD_PR_DOCKER_IMAGES.ymlinstalls it unpinned (npm install --global element-templates-cli), butrun-ai-agent-cpt(the job actually being replicated) pins it via.github/workflows/package.json. Copy the pinned form fromrun-ai-agent-cpt, not the unpinned one fromBUILD_PR_DOCKER_IMAGES.yml.-pl ... -amreactor double-build risk — buildingbundle/default-bundlewith--also-makeand then separately runningverify -pl connectors-e2e-test-agentic-ai -amin the same job risks building parts of the reactor twice. Worth usinginstall -DskipTests -DskipChecksonce for the modules both steps need, then a plainverify -pl <module>without-amfor the test step, mirroring whatE2E_BRANCH_RUN.ymlalready does deliberately for the same reason.camunda.process-test.connectors-secrets. GitHub's log masking is exact-string match only — sub-fields of a JSON blob (private_key_id,client_email, PEM body lines) aren't individually registered as secrets, so a container-log dump on test failure could leak them unmasked. This predates and is independent of the new PR-trigger workflow itself, but is a real gap in how test(agentic-ai): add Google Vertex AI real-LLM coverage and parameterise the E2E suite over providers #8226 already handles that credential inrun-ai-agent-cpttoday. Consider decoding to a file with restricted permissions instead of an env var, or GCP Workload Identity Federation via OIDC to drop the long-lived key entirely — flagging for separate follow-up, out of scope for this ticket.run-ai-agent-cptcurrently ends with a step calling./.github/actions/observe-build-status— that action directory does not exist in this repo (.github/actions/only containscodeowners-slack-mentionsandpost-failure-feedback), confirmed via a live completed run's job log showing##[error]Can't find 'action.yml'...— masked because the step hascontinue-on-error: true. This is a separate, pre-existing bug inrun-ai-agent-cpt/MERGE_QUEUE_HELM_TEST.yaml, worth its own ticket, and NOT something the new PR-triggered workflow should copy.post-failure-feedbackis the working sibling action already used elsewhere in the same file — if failure feedback on the PR is wanted, use that pattern instead.[labeled, synchronize]-triggered workflow produces no check run at all on unlabeled PRs — making it a required check would block every unlabeled PR forever. This is a manual branch-protection setting, called out here so it isn't set by accident.permissions:block omitted from the plan. Add job-levelcontents: read(matchingBUILD_PR_DOCKER_IMAGES.yml); addpull-requests: writeonly if a PR-comment feedback step is added per point 7.Confirmed correct on review: default-bundle-only is right (the SaaS bundle is genuinely unused by the real-LLM CPT suite —
application-it-real-llm.ymlonly referencesconnectors-bundle), linux/amd64-only is correct (the runner is amd64), and theNIGHTLY_E2E.ymlfork-gate bug is real exactly as described in point (b) above.Still open
timeout-minutesvalue once a first real run's timing is measured in the new single-job structure (see point (d) above).post-failure-feedback) or leave failures visible only via the check run itself.NIGHTLY_E2E.ymlfork-gate precedence bug (independent of this ticket)../.github/actions/observe-build-statusreference inMERGE_QUEUE_HELM_TEST.yaml(independent of this ticket).References
.github/workflows/MERGE_QUEUE_HELM_TEST.yaml(run-ai-agent-cptjob,on:triggers).github/workflows/BUILD_PR_DOCKER_IMAGES.yml.github/workflows/NIGHTLY_E2E.ymlGoogleVertexAiE2ETestIT) this new workflow should also coverGenerated by Claude Code