Skip to content

feat: gate agents tag on functional test validation - #6223

Open
maruiz93 wants to merge 1 commit into
fullsend-ai:mainfrom
maruiz93:gate-agents-tag
Open

feat: gate agents tag on functional test validation#6223
maruiz93 wants to merge 1 commit into
fullsend-ai:mainfrom
maruiz93:gate-agents-tag

Conversation

@maruiz93

Copy link
Copy Markdown
Contributor

Summary

  • Run agents functional tests against the release tag before pushing the version tag to fullsend-ai/agents
  • Extracts the agents tag push into a separate tag-agents job, gated on a new validate-agents job
  • The fullsend release (binary, checksums, v0 tag) ships regardless — only the agents tag is gated on test success

How it works

The release job is unchanged. Two new jobs:

  1. validate-agents — calls fullsend-ai/agents/.github/workflows/functional-tests.yml@main with fullsend_ref set to the release tag. This builds fullsend from the tag and runs agents' functional test suite against it.
  2. tag-agentsneeds: [release, validate-agents]. Only runs if validation passes. Contains the existing app-token generation and tag-push logic, moved from the release job.

Prerequisites

Test plan

  • Verify workflow YAML is valid (pre-commit actionlint passed)
  • Trigger a pre-release tag to validate the end-to-end flow
  • Confirm fullsend release completes even if agents validation fails

Closes #6173

🤖 Generated with Claude Code

@maruiz93
maruiz93 requested a review from a team as a code owner August 14, 2026 12:58
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 14, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:59 PM UTC · Completed 1:14 PM UTC

Commit: dcc245e · View workflow run →

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Gate agents tag push on agents functional tests

✨ Enhancement ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Run agents functional tests against the new release tag before tagging agents.
• Split cross-repo tagging into a dedicated job gated by test validation.
• Keep fullsend release artifacts and v0 floating tag independent of agents validation.
Diagram

graph TD
  A["release job"] --> B["validate-agents job"] --> C["agents functional-tests reusable workflow"]
  A --> D["tag-agents job"] --> E["push tag to agents"]
  B --> D
  C --> F["fullsend-ai/agents repo"]
  E --> F
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Run agents tests by checking out agents repo in this workflow
  • ➕ Keeps all logic in one workflow file and one runner context
  • ➕ Easier to version-pin agents test runner code (checkout SHA)
  • ➖ Requires duplicating/maintaining the agents functional test invocation here
  • ➖ More credentials/secrets surface area in this repo’s workflow
2. Trigger agents validation via repository_dispatch and wait for status
  • ➕ Decouples validation from a pinned reusable workflow interface
  • ➕ Can leverage agents-side required checks and richer reporting
  • ➖ More complex orchestration (dispatch payloads, polling/status gating)
  • ➖ Harder to make the workflow deterministic and debuggable

Recommendation: The chosen approach (calling the agents reusable functional test workflow via workflow_call and gating only the tag push) is the best tradeoff: it centralizes test logic in the agents repo, keeps the fullsend release path unchanged, and adds a clear dependency gate with minimal orchestration complexity. Consider pinning the reusable workflow ref to a commit/semantic tag instead of @main to avoid unexpected behavior changes during releases.

Files changed (1) +16 / -5

Other (1) +16 / -5
release.ymlGate agents tag sync behind cross-repo functional test validation +16/-5

Gate agents tag sync behind cross-repo functional test validation

• Adds a validate-agents job that invokes fullsend-ai/agents functional tests against the just-pushed release tag. Moves the agents tag push into a separate tag-agents job that depends on both release and validate-agents, ensuring the tag only propagates on test success while the fullsend release still ships.

.github/workflows/release.yml

@qodo-code-review

qodo-code-review Bot commented Aug 14, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Validate job overprivileged ✓ Resolved 🐞 Bug ⛨ Security
Description
The new validate-agents job calls a cross-repo reusable workflow without a job-level permissions
override, so it inherits the workflow-wide contents: write and id-token: write permissions. This
allows whatever code is in the called workflow to use this repo’s elevated GITHUB_TOKEN/OIDC
capabilities during a release run (impact depends on branch/tag protections and OIDC trust config).
Code

.github/workflows/release.yml[R55-58]

+  validate-agents:
+    needs: release
+    uses: fullsend-ai/agents/.github/workflows/functional-tests.yml@main
+    with:
Evidence
release.yml grants write-level repository and OIDC permissions at the workflow level, and the
newly-added validate-agents job does not override them while executing a reusable workflow from
another repository.

.github/workflows/release.yml[8-11]
.github/workflows/release.yml[55-60]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`validate-agents` invokes a reusable workflow from another repository but currently inherits the workflow-level permissions (`contents: write`, `id-token: write`). This unnecessarily grants elevated capabilities to the called workflow code.

## Issue Context
Workflow-level permissions are broad to support the `release` job, but `validate-agents` should run with least privilege.

## Fix Focus Areas
- .github/workflows/release.yml[8-11]
- .github/workflows/release.yml[55-60]

## Proposed fix
Add a job-level `permissions:` block to `validate-agents` (e.g., `contents: read`) so it does not inherit `contents: write`/`id-token: write`. Optionally, also consider moving the workflow-level permissions to the `release` job only (set workflow-level to read-only; override `release` with write/id-token).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Reusable workflow pinned to main ✓ Resolved 🐞 Bug ⛨ Security
Description
validate-agents references fullsend-ai/agents reusable workflow at @main, so future upstream
changes can silently change what runs in this release gate. This makes releases non-reproducible and
increases supply-chain risk (especially relevant in release workflows).
Code

.github/workflows/release.yml[R56-58]

+    needs: release
+    uses: fullsend-ai/agents/.github/workflows/functional-tests.yml@main
+    with:
Evidence
The new job explicitly uses ...functional-tests.yml@main, which is a mutable reference.

.github/workflows/release.yml[55-59]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The reusable workflow reference uses a mutable branch ref (`@main`), allowing unreviewed upstream changes to alter behavior of the release gate.

## Issue Context
This job runs as part of the release workflow; stability and provenance matter.

## Fix Focus Areas
- .github/workflows/release.yml[55-59]

## Proposed fix
Replace `@main` with an immutable reference (prefer a full commit SHA; a protected, versioned tag is also better than a branch). Establish a small process to periodically update the pinned SHA via a PR when you want upstream changes.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can describe a rule in plain language on the Rules page and Qodo drafts it for you

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread .github/workflows/release.yml
Comment thread .github/workflows/release.yml
@codecov

codecov Bot commented Aug 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 14, 2026

Copy link
Copy Markdown

Review

Findings

Medium

Low

  • [secret-exposure] .github/workflows/release.yml:67EVAL_GH_TOKEN is forwarded to the external reusable workflow fullsend-ai/agents/.github/workflows/functional-tests.yml. This secret name suggests a GitHub PAT with permissions beyond what GITHUB_TOKEN provides. The callee workflow is SHA-pinned to a specific commit (a8566cd) in an organization-owned repository, which limits supply-chain risk. However, the scope and permissions of EVAL_GH_TOKEN cannot be verified from this repository alone.
    Remediation: Document the required scope of EVAL_GH_TOKEN and verify it is a fine-grained PAT (or GitHub App token) with minimal permissions.

  • [architectural-coherence] .github/workflows/release.yml:60 — The validate-agents job calls fullsend-ai/agents/.github/workflows/functional-tests.yml pinned to a specific SHA (@a8566cd5305fe094b96588690118022967ad0061). While SHA-pinning follows the project's security convention, it creates version-skew risk for tightly-coupled sister repositories. If the agents workflow updates its interface, this workflow will continue calling the pinned old version until manually updated.
    Remediation: Document the SHA-pinning rationale and establish a process to bump the SHA when fullsend-ai/agents updates functional-tests.yml.

  • [workflow-conventions] .github/workflows/release.yml:55 — The validate-agents job does not set timeout-minutes. Per CI workflow conventions, every non-reusable workflow job must set timeout-minutes. The sibling tag-agents job sets timeout-minutes: 5, suggesting this was an oversight. Without an explicit timeout, the job defaults to GitHub's 6-hour limit.
    Remediation: Add timeout-minutes: to the validate-agents job with a value appropriate for functional test duration (e.g., 30–60 minutes).

Previous run

Review

Findings

Medium

Low

  • [secret-exposure] .github/workflows/release.yml:64EVAL_GH_TOKEN is forwarded to the external reusable workflow fullsend-ai/agents/.github/workflows/functional-tests.yml. This secret name suggests a GitHub PAT with permissions beyond what GITHUB_TOKEN provides. The callee workflow is SHA-pinned to a specific commit (a8566cd) in an organization-owned repository, which limits supply-chain risk. However, the scope and permissions of EVAL_GH_TOKEN cannot be verified from this repository alone.
    Remediation: Document the required scope of EVAL_GH_TOKEN and verify it is a fine-grained PAT (or GitHub App token) with minimal permissions.
Previous run (2)

Review

Findings

Medium

  • [secret-exposure] .github/workflows/release.yml:62secrets: inherit on the validate-agents job passes every secret available to the workflow — including RELEASE_APP_PRIVATE_KEY (a long-lived GitHub App PEM key) and SLACK_WEBHOOK_URL — to the external reusable workflow fullsend-ai/agents/.github/workflows/functional-tests.yml. The callee is SHA-pinned and organization-owned, limiting supply-chain risk. However, the repo's own workflow-contracts documentation explicitly states "do not use secrets: inherit as a substitute for explicit forwarding." Functional tests are unlikely to need the release signing key or the Slack webhook. Replacing with explicit secret enumeration aligns with documented conventions and reduces blast radius if new secrets are added to the repo.
    Remediation: Replace secrets: inherit with an explicit secrets: block that passes only the secrets the functional-tests workflow actually requires. If it needs none, omit the secrets: key entirely. If it needs specific secrets for WIF or test infrastructure, enumerate them.

  • [protected-path] .github/workflows/release.yml — This PR modifies .github/workflows/release.yml, which is under the protected path .github/. The PR links to issue Gate agents version tag on functional test validation at release time #6173 and provides rationale for the change. Human approval is always required for protected-path changes, regardless of context.

Previous run (3)

Review

Findings

Medium

  • [permission-expansion] .github/workflows/release.yml:55 — The validate-agents job calls a reusable workflow in fullsend-ai/agents without declaring job-level permissions. It inherits the workflow-level contents: write and id-token: write, granting the external reusable workflow a GITHUB_TOKEN with write access to this repository's contents. Per CI Workflows guidance: "Set permissions: {} at the workflow level and grant only the permissions each job needs at the job level."
    Remediation: Add a job-level permissions block to validate-agents scoped to what the functional tests require (likely contents: read or permissions: {}).

  • [missing-required-field] .github/workflows/release.yml:61 — The tag-agents job does not set timeout-minutes. Per CI Workflows guidance, every non-reusable workflow job must set timeout-minutes. Without it, the job inherits GitHub's default 6-hour timeout. The job only generates a token and makes two API calls, so a short timeout is appropriate.
    Remediation: Add timeout-minutes: 5 to the tag-agents job definition.

  • [protected-path] .github/workflows/release.yml — This PR modifies .github/workflows/release.yml, which is under the protected path .github/. The PR links to issue Gate agents version tag on functional test validation at release time #6173 and provides rationale for the change. Human approval is always required for protected-path changes, regardless of context.


Labels: PR modifies CI release workflow under .github/workflows/

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment component/ci CI pipelines and checks labels Aug 14, 2026

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional finding that falls outside the visible diff hunk (unchanged line, so no inline position is available):

[MEDIUM] TOCTOU: tag-agents re-resolves agents' main SHA independently instead of tagging the commit that validate-agents actually tested.github/workflows/release.yml:85

tag-agents (needs: [release, validate-agents]) resolves the tag target at push time via AGENTS_SHA=$(gh api repos/fullsend-ai/agents/git/ref/heads/main --jq '.object.sha') (line 85) — a fresh, independent query of agents' main HEAD made after validate-agents has already completed. validate-agents's functional-tests job checks out agents at github.workflow_sha (functional-tests.yml lines 165-167, 219-221) at the time that run started. If a new commit lands on fullsend-ai/agents:main in the window between when validate-agents starts testing and when tag-agents runs (functional tests run with a per-matrix-leg timeout that leaves a real window), or if tag-agents is manually re-run later, the pushed tag ends up on a commit of fullsend-ai/agents that was never covered by the validation this PR adds — undermining the PR's core stated guarantee that the agents tag only moves after passing validation for that exact commit.

Suggestion: have the tested commit flow through explicitly instead of being re-resolved — add a workflow_call output to fullsend-ai/agents/.github/workflows/functional-tests.yml (e.g. tested_sha) surfacing the SHA it actually checked out and tested, and have tag-agents tag needs.validate-agents.outputs.tested_sha instead of re-querying heads/main. That functional-tests.yml change belongs in fullsend-ai/agents, so this may need a small follow-up PR there, with tag-agents here updated to consume the new output.

# pre-release semantics. Intentionally coupled to the v0 step:
# if the v0 move fails, the release state is suspect and agents
# should not be tagged until the issue is investigated.
validate-agents:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[CRITICAL] validate-agents never forwards secrets — functional tests silently no-op, gate always passes

Confirmed at head dcc245ee: the validate-agents job (lines 55-59) calls fullsend-ai/agents/.github/workflows/functional-tests.yml@main with only a with: fullsend_ref block — no secrets: key at all. functional-tests.yml on fullsend-ai/agents@main declares E2E_GCP_WIF_PROVIDER, E2E_GCP_SERVICE_ACCOUNT, E2E_GCP_PROJECT_ID, EVAL_GH_TOKEN all as required: false (lines 38-46), so the call succeeds syntactically with every secret resolving empty. Its "Check for secrets" step (lines 332-340) reads secrets.E2E_GCP_WIF_PROVIDER, finds it empty, sets available=false, which gates off GCP auth, "Run functional tests", and results upload via if: steps.secrets-check.outputs.available == 'true' — but does not fail the job.

The safety net doesn't catch this either: functional-tests-complete (lines 398-425) only errors when CROSS_REPO == true && TESTS_RESULT == 'skipped', i.e. only when the whole functional-tests job is skipped at the job level (e.g. empty matrix from detect). Here the job still runs and completes with result success — only its internal steps are individually skipped — so this guard never fires.

Net effect: validate-agents always reports success without a single functional test executing, and tag-agents proceeds unconditionally, defeating the PR's stated purpose.

Suggestion: Add secrets: inherit to the validate-agents job (or explicitly map the four named secrets: E2E_GCP_WIF_PROVIDER, E2E_GCP_SERVICE_ACCOUNT, E2E_GCP_PROJECT_ID, EVAL_GH_TOKEN). After fixing, verify via an actual pre-release tag run that the "Run functional tests" step actually executes (not skipped) — a green job with all internal steps silently skipped looks identical to a real pass in the Actions UI.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in f319418 — added secrets: inherit and scoped permissions to contents: read + id-token: write (the latter required for GCP OIDC auth in the called workflow). Also pinned the workflow ref to a commit SHA.

# pre-release semantics. Intentionally coupled to the v0 step:
# if the v0 move fails, the release state is suspect and agents
# should not be tagged until the issue is investigated.
validate-agents:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] If applying the flagged permission-narrowing fix, keep id-token: write or GCP OIDC auth in the called workflow breaks

The existing bot reviews on this line (qodo-code-review, fullsend-ai-review) correctly flag that validate-agents has no job-level permissions: block and inherits the workflow-level contents: write, id-token: write. However, the suggested remediation from fullsend-ai-review is ambiguous/incomplete: "scoped to what functional tests require (likely contents: read or permissions: {})".

functional-tests.yml (fullsend-ai/agents@main) declares permissions: { contents: read, id-token: write } on its functional-tests job (lines 208-210) and uses id-token: write via google-github-actions/auth for GCP WIF authentication. Per GitHub's reusable-workflow permission model, the calling job's permissions: block caps what the entire called workflow can use — if the fix applied here is permissions: {} or a bare contents: read, GCP WIF auth in the called workflow will fail once the secrets-forwarding bug (see the CRITICAL comment on this same line) is fixed and the auth step is actually reached.

Suggestion: When addressing the already-flagged permission-expansion finding, use permissions: { contents: read, id-token: write } on validate-agents, not a blanket contents: read or permissions: {} — otherwise the fix for one bug (over-broad permissions) reintroduces the CRITICAL bug (tests never really running) in a different form once secrets are correctly forwarded.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in f319418 — used permissions: { contents: read, id-token: write } as suggested.

# pre-release semantics. Intentionally coupled to the v0 step:
# if the v0 move fails, the release state is suspect and agents
# should not be tagged until the issue is investigated.
validate-agents:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] Gating design shipped without ever being exercised end-to-end — PR's own test plan confirms this

The PR's "Test plan" section lists all three items unchecked: [ ] Verify workflow YAML is valid (pre-commit actionlint passed), [ ] Trigger a pre-release tag to validate the end-to-end flow, [ ] Confirm fullsend release completes even if agents validation fails. Only actionlint (a syntax check) is claimed done, and even that box is literally unchecked in the markdown. The CRITICAL secrets-forwarding bug (see comment above) is direct, concrete evidence of the risk this represents: a syntactically valid workflow that never runs real tests passes exactly the checks that were actually performed (actionlint), while looking identical to a working gate in the Actions UI. Other unverified cross-repo assumptions (secret availability/scoping in this repo, how github.workflow_sha resolves for a cross-org workflow_call) compound the risk of the design failing either open (silent pass, confirmed) or closed (blocking every release) once actually run.

Suggestion: Before merging (or immediately after, behind a real pre-release tag as the test plan itself proposes), trigger an actual pre-release tag and confirm from the Actions run logs that "Run functional tests" executes (not skipped) inside validate-agents, and separately confirm tag-agents is blocked when validation is forced to fail. Check off the test-plan boxes only once genuinely observed, not once the job goes green.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed — will trigger a pre-release tag run to validate the full flow before merging. The secrets-forwarding fix in f319418 should make this meaningful now.

@maruiz93

Copy link
Copy Markdown
Contributor Author

Tracked as fullsend-ai/agents#800 — will add a tested_sha output to agents' functional-tests.yml and update tag-agents here to consume it.

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 14, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:02 PM UTC · Completed 3:15 PM UTC

Commit: f319418 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 14, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 3:34 PM UTC · Ended 3:36 PM UTC

Commit: 147fdb1 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 14, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:37 PM UTC · Completed 3:49 PM UTC

Commit: 4b6341c · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

Run agents functional tests against the release tag before pushing
the version tag to fullsend-ai/agents. The fullsend release itself
ships regardless — only the agents tag is gated on test success.

Closes fullsend-ai#6173
Ref: fullsend-ai/agents#775

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Marta Anon <manon@redhat.com>
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 14, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:52 PM UTC · Completed 4:07 PM UTC

Commit: a1368f5 · View workflow run →

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

E2E_GCP_WIF_PROVIDER: ${{ secrets.E2E_GCP_WIF_PROVIDER }}
E2E_GCP_SERVICE_ACCOUNT: ${{ secrets.E2E_GCP_SERVICE_ACCOUNT }}
E2E_GCP_PROJECT_ID: ${{ secrets.E2E_GCP_PROJECT_ID }}
EVAL_GH_TOKEN: ${{ secrets.EVAL_GH_TOKEN }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] secret-exposure

EVAL_GH_TOKEN is forwarded to the external reusable workflow fullsend-ai/agents/.github/workflows/functional-tests.yml. This secret name suggests a GitHub PAT with permissions beyond what GITHUB_TOKEN provides. The callee workflow is SHA-pinned to a specific commit (a8566cd) in an organization-owned repository, which limits supply-chain risk. However, the scope and permissions of EVAL_GH_TOKEN cannot be verified from this repository alone.

Suggested fix: Document the required scope of EVAL_GH_TOKEN and verify it is a fine-grained PAT (or GitHub App token) with minimal permissions.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already addressed in a prior comment — EVAL_GH_TOKEN is an existing agents repo secret managed on the agents side.

permissions:
contents: read
id-token: write
uses: fullsend-ai/agents/.github/workflows/functional-tests.yml@a8566cd5305fe094b96588690118022967ad0061 # main

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] architectural-coherence

The validate-agents job calls fullsend-ai/agents/.github/workflows/functional-tests.yml pinned to a specific SHA (@a8566cd5305fe094b96588690118022967ad0061). While SHA-pinning follows the project's security convention, it creates version-skew risk for tightly-coupled sister repositories. If the agents workflow updates its interface, this workflow will continue calling the pinned old version until manually updated.

Suggested fix: Document the SHA-pinning rationale and establish a process to bump the SHA when fullsend-ai/agents updates functional-tests.yml.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SHA pinning was done per reviewer request in the first round — it's intentional for supply-chain safety in the release workflow.

# pre-release semantics. Intentionally coupled to the v0 step:
# if the v0 move fails, the release state is suspect and agents
# should not be tagged until the issue is investigated.
validate-agents:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] workflow-conventions

The validate-agents job does not set timeout-minutes. Per CI workflow conventions, every non-reusable workflow job must set timeout-minutes. The sibling tag-agents job sets timeout-minutes: 5, suggesting this was an oversight. Without an explicit timeout, the job defaults to GitHub's 6-hour limit.

Suggested fix: Add timeout-minutes: to the validate-agents job with a value appropriate for functional test duration (e.g., 30-60 minutes).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validate-agents is a reusable workflow call (uses:), not a regular job — timeout-minutes is controlled by the called workflow's own job definitions, not the caller.

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional finding that doesn't map to a changed line in this PR's diff (target file is renovate.json, untouched here), so posting it in the review body:

[HIGH] New fullsend-ai/agents workflow pin is invisible to Renovate and already 26 commits stalerenovate.json:29

The new uses: fullsend-ai/agents/.github/workflows/functional-tests.yml@a8566cd... pin added by this PR (release.yml:60) falls under renovate.json's existing packageRule: {"description": "Ignore fullsend self-references (own reusable workflows and actions)", "matchManagers": ["github-actions"], "matchPackageNames": ["/^fullsend-ai\\//"], "enabled": false}. Renovate's github-actions manager resolves this dependency's package name to fullsend-ai/agents, which matches that regex, so it will never receive automated bump PRs. I verified live that agents' main is already 26 commits ahead of the pinned SHA (gh api repos/fullsend-ai/agents/compare/a8566cd5305fe094b96588690118022967ad0061...mainahead_by: 26). This is separate from the already-tracked tested-vs-tagged commit mismatch (agents#800): even after that fix lands, the value of the gate depends on the pin being refreshed periodically, and there is currently zero automated mechanism or process to do so — meaning validate-agents will keep testing an ever-more-stale snapshot of agents indefinitely with no signal to anyone.

Failure scenario: Over the coming months, agents' main accumulates dozens/hundreds of commits while the pin in release.yml never moves (no renovate PR, no reminder). Every future release's validate-agents job keeps testing the same frozen a8566cd snapshot, so the "gate agents tag on functional test validation" feature silently degrades into testing code that no longer resembles what's about to be tagged, defeating the PR's stated purpose (closes #6173) without any error or warning ever appearing.

Suggestion: Add a packageRule exception scoped to this specific file/dependency (e.g. matchFileNames: ['.github/workflows/release.yml'] or a more specific packageName match) that re-enables Renovate tracking for this one cross-repo pin, while keeping the existing ignore rule for true same-repo self-references. Alternatively, add a scheduled reminder/check that flags when the pin falls more than N commits behind agents main.

E2E_GCP_PROJECT_ID: ${{ secrets.E2E_GCP_PROJECT_ID }}
EVAL_GH_TOKEN: ${{ secrets.EVAL_GH_TOKEN }}

tag-agents:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] No alerting when the agents tag sync (validate-agents/tag-agents) fails or is skipped

By design, the fullsend release (GoReleaser artifacts, v0 floating tag) completes independently of the validate-agents/tag-agents outcome. I read the full workflow and confirmed the only use of SLACK_WEBHOOK_URL is in the release job's GoReleaser step for release announcements — there is no step anywhere that notifies (Slack, issue, etc.) if validate-agents fails or if tag-agents is skipped/fails to push the tag to fullsend-ai/agents.

Failure scenario: A fullsend release ships successfully, but agents functional tests fail (or the cross-repo tag push errors for an unrelated reason, e.g. GitHub App token/permission issue). No one is notified. fullsend-ai/agents silently falls behind the released fullsend version with no signal until a human notices the missing tag much later, potentially after other work has already built on the assumption that agents was tagged.

Suggestion: Add a rollup step/job with if: failure() on needs: [validate-agents, tag-agents] that posts to the existing Slack webhook (or opens/updates a tracking issue) so a missed or failed agents tag sync is visible immediately rather than discovered later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/ci CI pipelines and checks requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gate agents version tag on functional test validation at release time

2 participants