Skip to content

docs(#5629): add reusable workflow carve-out for github.workflow in concurrency groups - #6443

Merged
ralphbean merged 6 commits into
fullsend-ai:mainfrom
shairevivo:srevivo/fix-5629-reusable-workflow-carveout
Aug 25, 2026
Merged

docs(#5629): add reusable workflow carve-out for github.workflow in concurrency groups#6443
ralphbean merged 6 commits into
fullsend-ai:mainfrom
shairevivo:srevivo/fix-5629-reusable-workflow-carveout

Conversation

@shairevivo

Copy link
Copy Markdown
Contributor

Summary

  • Add exception to the concurrency groups section of docs/contributing/ci-workflows.md noting that reusable workflows (on: workflow_call) must use hardcoded role-specific prefixes instead of ${{ github.workflow }}
  • github.workflow resolves to the caller's name in workflow_call context, so using it would produce incorrect concurrency scoping
  • Extends the "Why" paragraph to explain the rationale for the exception

Fixes #5629

Test plan

  • Read the updated concurrency groups section and confirm it accurately describes both patterns
  • Verify the exception matches the repo's actual reusable workflows (e.g., reusable-code.yml uses fullsend-code-agent-, not ${{ github.workflow }})

🤖 Generated with Claude Code

…rkflow in concurrency groups

The concurrency groups section stated github.workflow should always be
used as the workflow identifier. In reusable workflows (on:
workflow_call), github.workflow resolves to the caller's name, not the
reusable workflow's own name. Add an exception noting that reusable
workflows must use hardcoded role-specific prefixes instead.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Shai Revivo <srevivo@redhat.com>
@shairevivo
shairevivo requested a review from a team as a code owner August 21, 2026 11:26
@github-actions

Copy link
Copy Markdown

E2E tests did not run

E2E tests run automatically for org/repo members and collaborators on pull requests.

For other contributors, a maintainer must add the ok-to-test label after the latest push.

See E2E testing guide for details.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Docs: clarify concurrency group naming for reusable workflows

📝 Documentation 🕐 Less than 10 minutes

Grey Divider

AI Description

• Document exception for reusable workflows: avoid ${{ github.workflow }} in concurrency groups.
• Explain github.workflow resolves to caller in workflow_call, risking wrong scoping.
• Extend rationale for why hardcoded prefixes are otherwise redundant.
Diagram

graph TD
  caller(["Caller workflow"]) --> ctx{{"`github.workflow` (workflow_call)"}} --> group["Concurrency group key"] --> runs[("In-flight runs")] --> outcome["Cancel/queue behavior"]
  reusable["Reusable workflow"] -- "Use hardcoded prefix" --> group

  subgraph Legend
    direction LR
    _w(["Workflow"]) ~~~ _d{{"Context value"}} ~~~ _g["Group key"] ~~~ _r[("Runs")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Add a concrete YAML example for reusable workflows
  • ➕ Removes ambiguity by showing the exact concurrency-group pattern to use in workflow_call
  • ➕ Makes it easier for contributors to copy/paste the safe approach
  • ➖ Adds verbosity to the conventions doc
  • ➖ Example may need updates if repo conventions change
2. Link to GitHub docs/issue about `github.workflow` behavior in `workflow_call`
  • ➕ Provides an authoritative reference and reduces future confusion
  • ➕ Helps justify the carve-out beyond local conventions
  • ➖ External docs may change or move
  • ➖ Still requires readers to connect the reference back to the repo’s convention

Recommendation: The PR’s approach is correct and appropriately scoped: it adds a targeted carve-out plus rationale, preventing incorrect concurrency scoping in reusable workflows. If reviewers think contributors will still misapply the rule, consider a small follow-up to add a short YAML snippet and/or an upstream reference link.

Files changed (1) +2 / -1

Documentation (1) +2 / -1
ci-workflows.mdDocument concurrency-group exception for 'workflow_call' workflows +2/-1

Document concurrency-group exception for 'workflow_call' workflows

• Adds an explicit exception stating reusable workflows must use hardcoded role-specific concurrency prefixes rather than '${{ github.workflow }}'. Expands the rationale to explain that 'github.workflow' resolves to the caller’s workflow name in 'workflow_call', which could accidentally share concurrency groups and cancel caller runs.

docs/contributing/ci-workflows.md

@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown

Site preview

Preview: https://48123c34-site.fullsend-ai.workers.dev

Commit: ad935d11bf28edbb9e59c471ff0bfaa39014f6c2

@codecov

codecov Bot commented Aug 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@qodo-code-review

qodo-code-review Bot commented Aug 21, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Informational

1. Docs conflict with workflows ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
The new "Exception" states reusable workflows (on: workflow_call) must not use `${{
github.workflow }}, but this repo has reusable workflows (e.g., e2e.yml, functional-tests.yml`)
that declare workflow_call and still use github.workflow in concurrency.group, making the
guidance internally inconsistent and likely to mislead future edits.
Code

docs/contributing/ci-workflows.md[R22-25]

+- **Exception:** reusable workflows (`on: workflow_call`) must use a hardcoded role-specific prefix (e.g., `fullsend-code-agent-`) instead of `${{ github.workflow }}`, because in `workflow_call` context `github.workflow` resolves to the *caller's* workflow name, not the reusable workflow's own name. Using it would produce incorrect concurrency scoping and could cancel the caller's runs.
- Never cancel in-progress runs on the default branch (`refs/heads/main`). Gate `cancel-in-progress` when the workflow triggers on `push` to `main`.

-**Why:** A hardcoded prefix like `my-workflow-${{ github.workflow }}` is redundant — `github.workflow` already resolves to the workflow `name:` field. The duplication creates a confusing group key and wastes characters.
+**Why:** A hardcoded prefix like `my-workflow-${{ github.workflow }}` is redundant — `github.workflow` already resolves to the workflow `name:` field. The duplication creates a confusing group key and wastes characters. The reusable-workflow exception exists because GitHub resolves `github.workflow` from the caller's context, so a reusable workflow using it would share a concurrency group with its caller.
Relevance

● Weak

Identical exception-for-reusable-workflows suggestion was explicitly rejected in this file's
history.

PR-#5523

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The docs change introduces a blanket rule for all workflow_call workflows, but the repo already
contains workflow_call-enabled workflows that use github.workflow in concurrency.group, so the
statement is inconsistent with existing implementations.

docs/contributing/ci-workflows.md[5-25]
.github/workflows/e2e.yml[45-63]
.github/workflows/functional-tests.yml[9-37]

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 docs claim that reusable workflows (`on: workflow_call`) must use a hardcoded prefix instead of `${{ github.workflow }}`. In this repo, some `workflow_call`-enabled workflows still use `github.workflow` in their concurrency expression, so the guidance is too broad / contradictory.

### Issue Context
This inconsistency makes it unclear what the actual convention is (e.g., whether it applies only to agent-style reusable workflows like `reusable-code.yml`, or to any workflow that includes `workflow_call`).

### Fix Focus Areas
- docs/contributing/ci-workflows.md[19-25]

Update the exception wording to match real patterns in-repo, e.g.:
- Clarify the exception applies to *agent reusable workflows* (or workflows intended to be called by multiple callers) where concurrency must be distinct from the caller.
- Optionally mention that mixed-trigger workflows (like `e2e.yml` / `functional-tests.yml`) may still use `${{ github.workflow }}` for non-`workflow_call` triggers, while using a hardcoded prefix for the `workflow_call` path if needed.
- Add/adjust examples to include both patterns found in-repo (agent-prefix style and `e2e-{PR}`/`functional-{PR}` style).

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


Grey Divider

Context sources
✅ Compliance rules (platform): 58 rules

Grey Divider

Tip of the day
💡 Did you know, you can hide the parts of a finding you never read, like the evidence or the agent prompt

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

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

Automated multi-agent review pass (independent parallel reviewers, findings cross-checked against the current head and against actual in-repo workflow files before posting).

[MEDIUM] Opening rule (line 7) directly contradicts the new Exception (line 22), outside GitHub's inline-comment diff range so noting it here: line 7 states the absolute rule "Use ${{ github.workflow }} as the workflow identifier — never duplicate the workflow name as a hardcoded string prefix." The new Exception then requires exactly what this forbids for workflow_call workflows, with no cross-reference between the two. Suggestion: append "...except for workflow_call reusable workflows (see exception below)" to line 7.

Remaining findings filed as inline comments below.

Comment thread docs/contributing/ci-workflows.md Outdated
Comment thread docs/contributing/ci-workflows.md Outdated
…ll pattern

Address review findings: narrow the exception to workflow_call-only
workflows (hybrid workflows like e2e.yml/functional-tests.yml are
not affected), show the full inputs.*-based concurrency pattern from
reusable-code.yml, and cross-reference from the opening rule.

Signed-off-by: Shai Revivo <srevivo@redhat.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Shai Revivo <srevivo@redhat.com>
@shairevivo

Copy link
Copy Markdown
Contributor Author

@rh-hemartin can you merge? I don't have rights :-)

@ralphbean
ralphbean added this pull request to the merge queue Aug 25, 2026
Merged via the queue into fullsend-ai:main with commit c4d5cbd Aug 25, 2026
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix ci-workflows.md: add reusable workflow carve-out for github.workflow

4 participants