diff --git a/.github/workflows/reusable-code.yml b/.github/workflows/reusable-code.yml index 5ed01ebaf..0cdbbbf23 100644 --- a/.github/workflows/reusable-code.yml +++ b/.github/workflows/reusable-code.yml @@ -1,5 +1,8 @@ -# Reusable code agent workflow. Called by thin callers in .fullsend repos -# via workflow_call. Runs in the caller's repo context (secrets, checkout). +# Reusable code agent workflow. Called by dispatch workflows via workflow_call. +# Runs in the caller's repo context (secrets, checkout). +# +# Concurrency: agent-scoped cancel-in-progress group (distinct from dispatch/thin +# caller groups so workflow_call parent runs are not cancelled). name: Code Agent on: @@ -39,6 +42,10 @@ on: FULLSEND_GCP_PROJECT_ID: required: true +concurrency: + group: fullsend-code-agent-${{ inputs.source_repo }}-${{ fromJSON(inputs.event_payload).issue.number }} + cancel-in-progress: true + jobs: code: name: Code diff --git a/.github/workflows/reusable-dispatch.yml b/.github/workflows/reusable-dispatch.yml index 95bf3cb4d..a0cf0d775 100644 --- a/.github/workflows/reusable-dispatch.yml +++ b/.github/workflows/reusable-dispatch.yml @@ -3,6 +3,11 @@ # workflow_call jobs. This is the per-repo equivalent of the per-org # dispatch.yml + thin caller pair. # +# Concurrency: each stage job declares a per-role cancel-in-progress dispatch group. +# Reusable stage workflows use distinct agent-scoped groups (fullsend-{stage}-agent-…) +# so workflow_call parents are not cancelled. Roles operate independently — review +# dispatches do not cancel triage, code, fix, etc. +# # Flow: shim (per-repo) → reusable-dispatch.yml → reusable-{stage}.yml # Nesting: 3 levels of workflow_call (within GitHub's 4-level limit) # @@ -360,6 +365,9 @@ jobs: name: Triage needs: route if: needs.route.outputs.stage == 'triage' + concurrency: + group: fullsend-triage-${{ github.repository }}-${{ github.event.issue.number }} + cancel-in-progress: true # @v0 is hardcoded — GHA does not support expressions in uses:. # fullsend_ai_ref controls the ref for composite actions inside stage workflows. uses: fullsend-ai/fullsend/.github/workflows/reusable-triage.yml@v0 @@ -380,6 +388,9 @@ jobs: name: Code needs: route if: needs.route.outputs.stage == 'code' + concurrency: + group: fullsend-code-${{ github.repository }}-${{ github.event.issue.number }} + cancel-in-progress: true uses: fullsend-ai/fullsend/.github/workflows/reusable-code.yml@v0 with: event_type: ${{ github.event_name }} @@ -398,6 +409,9 @@ jobs: name: Review needs: route if: needs.route.outputs.stage == 'review' + concurrency: + group: fullsend-review-${{ github.repository }}-${{ github.event.pull_request.number || github.event.issue.number }} + cancel-in-progress: true uses: fullsend-ai/fullsend/.github/workflows/reusable-review.yml@v0 with: event_type: ${{ github.event_name }} @@ -416,6 +430,9 @@ jobs: name: Fix needs: route if: needs.route.outputs.stage == 'fix' + concurrency: + group: fullsend-fix-${{ github.repository }}-${{ github.event.pull_request.number || github.event.issue.number }} + cancel-in-progress: true uses: fullsend-ai/fullsend/.github/workflows/reusable-fix.yml@v0 with: event_type: ${{ github.event_name }} @@ -435,6 +452,9 @@ jobs: name: Retro needs: route if: needs.route.outputs.stage == 'retro' + concurrency: + group: fullsend-retro-${{ github.repository }}-${{ github.event.pull_request.number || github.event.issue.number }} + cancel-in-progress: true uses: fullsend-ai/fullsend/.github/workflows/reusable-retro.yml@v0 with: event_type: ${{ github.event_name }} @@ -453,6 +473,9 @@ jobs: name: Prioritize needs: route if: needs.route.outputs.stage == 'prioritize' + concurrency: + group: fullsend-prioritize-${{ github.repository }}-${{ github.event.issue.number }} + cancel-in-progress: true uses: fullsend-ai/fullsend/.github/workflows/reusable-prioritize.yml@v0 with: event_type: ${{ github.event_name }} diff --git a/.github/workflows/reusable-fix.yml b/.github/workflows/reusable-fix.yml index 1f75a6c54..2f41130b3 100644 --- a/.github/workflows/reusable-fix.yml +++ b/.github/workflows/reusable-fix.yml @@ -1,5 +1,8 @@ -# Reusable fix agent workflow. Called by thin callers in .fullsend repos -# via workflow_call. Runs in the caller's repo context (secrets, checkout). +# Reusable fix agent workflow. Called by dispatch workflows via workflow_call. +# Runs in the caller's repo context (secrets, checkout). +# +# Concurrency: agent-scoped cancel-in-progress group (distinct from dispatch/thin +# caller groups so workflow_call parent runs are not cancelled). name: Fix Agent on: @@ -51,6 +54,15 @@ on: FULLSEND_GCP_PROJECT_ID: required: true +concurrency: + group: >- + fullsend-fix-agent-${{ inputs.source_repo }}-${{ + fromJSON(inputs.event_payload).pull_request.number + || fromJSON(inputs.event_payload).issue.number + || inputs.pr_number + }} + cancel-in-progress: true + jobs: fix: name: Fix diff --git a/.github/workflows/reusable-prioritize.yml b/.github/workflows/reusable-prioritize.yml index 8cfac73fb..a9216c833 100644 --- a/.github/workflows/reusable-prioritize.yml +++ b/.github/workflows/reusable-prioritize.yml @@ -1,5 +1,8 @@ -# Reusable prioritize agent workflow. Called by thin callers in .fullsend repos -# via workflow_call. Runs in the caller's repo context (secrets, checkout). +# Reusable prioritize agent workflow. Called by dispatch workflows via workflow_call. +# Runs in the caller's repo context (secrets, checkout). +# +# Concurrency: agent-scoped cancel-in-progress group (distinct from dispatch/thin +# caller groups so workflow_call parent runs are not cancelled). name: Prioritize Agent on: @@ -43,6 +46,10 @@ on: FULLSEND_GCP_PROJECT_ID: required: true +concurrency: + group: fullsend-prioritize-agent-${{ inputs.source_repo }}-${{ fromJSON(inputs.event_payload).issue.number }} + cancel-in-progress: true + jobs: prioritize: name: Prioritize diff --git a/.github/workflows/reusable-retro.yml b/.github/workflows/reusable-retro.yml index 92edf04c1..05f86ad92 100644 --- a/.github/workflows/reusable-retro.yml +++ b/.github/workflows/reusable-retro.yml @@ -1,5 +1,8 @@ -# Reusable retro agent workflow. Called by thin callers in .fullsend repos -# via workflow_call. Runs in the caller's repo context (secrets, checkout). +# Reusable retro agent workflow. Called by dispatch workflows via workflow_call. +# Runs in the caller's repo context (secrets, checkout). +# +# Concurrency: agent-scoped cancel-in-progress group (distinct from dispatch/thin +# caller groups so workflow_call parent runs are not cancelled). name: Retro Agent on: @@ -39,6 +42,10 @@ on: FULLSEND_GCP_PROJECT_ID: required: true +concurrency: + group: fullsend-retro-agent-${{ inputs.source_repo }}-${{ fromJSON(inputs.event_payload).pull_request.number || fromJSON(inputs.event_payload).issue.number }} + cancel-in-progress: true + jobs: retro: name: Retro diff --git a/.github/workflows/reusable-review.yml b/.github/workflows/reusable-review.yml index 2f3159fb1..02d4c4df7 100644 --- a/.github/workflows/reusable-review.yml +++ b/.github/workflows/reusable-review.yml @@ -1,5 +1,8 @@ -# Reusable review agent workflow. Called by thin callers in .fullsend repos -# via workflow_call. Runs in the caller's repo context (secrets, checkout). +# Reusable review agent workflow. Called by dispatch workflows via workflow_call. +# Runs in the caller's repo context (secrets, checkout). +# +# Concurrency: agent-scoped cancel-in-progress group (distinct from dispatch/thin +# caller groups so workflow_call parent runs are not cancelled). name: Review Agent on: @@ -39,6 +42,10 @@ on: FULLSEND_GCP_PROJECT_ID: required: true +concurrency: + group: fullsend-review-agent-${{ inputs.source_repo }}-${{ fromJSON(inputs.event_payload).pull_request.number || fromJSON(inputs.event_payload).issue.number }} + cancel-in-progress: true + jobs: review: name: Review diff --git a/.github/workflows/reusable-triage.yml b/.github/workflows/reusable-triage.yml index af1dedbf6..52f4c982b 100644 --- a/.github/workflows/reusable-triage.yml +++ b/.github/workflows/reusable-triage.yml @@ -1,5 +1,9 @@ -# Reusable triage agent workflow. Called by thin callers in .fullsend repos -# via workflow_call. Runs in the caller's repo context (secrets, checkout). +# Reusable triage agent workflow. Called by dispatch workflows via workflow_call. +# Runs in the caller's repo context (secrets, checkout). +# +# Concurrency: agent-scoped cancel-in-progress group (distinct from dispatch/thin +# caller groups so workflow_call parent runs are not cancelled). Latest agent +# run for an issue wins. name: Triage Agent on: @@ -39,6 +43,10 @@ on: FULLSEND_GCP_PROJECT_ID: required: true +concurrency: + group: fullsend-triage-agent-${{ inputs.source_repo }}-${{ fromJSON(inputs.event_payload).issue.number }} + cancel-in-progress: true + jobs: triage: name: Triage diff --git a/docs/ADRs/0034-centralized-shim-routing-via-dispatch.md b/docs/ADRs/0034-centralized-shim-routing-via-dispatch.md index 6884b1c24..bff1351da 100644 --- a/docs/ADRs/0034-centralized-shim-routing-via-dispatch.md +++ b/docs/ADRs/0034-centralized-shim-routing-via-dispatch.md @@ -162,3 +162,8 @@ The `stage` input to `dispatch.yml` becomes optional. When provided the same routing script. Per-org shims could also adopt `reusable-fullsend.yml` directly, eliminating `dispatch.yml` as a routing layer entirely — see ADR 0033 Open Questions. + +## Implementation note (#981) + +Per-role dispatch concurrency is configured in repository workflows; the routing +model in this ADR is unchanged. diff --git a/docs/ADRs/0041-synchronous-workflow-call-event-dispatch.md b/docs/ADRs/0041-synchronous-workflow-call-event-dispatch.md index 0d43276db..8b6ed9560 100644 --- a/docs/ADRs/0041-synchronous-workflow-call-event-dispatch.md +++ b/docs/ADRs/0041-synchronous-workflow-call-event-dispatch.md @@ -138,3 +138,8 @@ re-evaluate whether a discovery mechanism is needed. `needs:` / concurrency review ([#504](https://github.com/fullsend-ai/fullsend/issues/504)). - Discovery may be revisited after [ADR 0038](0038-universal-harness-access.md) agent architecture changes land. + +## Implementation note (#981) + +Per-repo concurrency follow-up ([#504](https://github.com/fullsend-ai/fullsend/issues/504)) +is addressed in workflow configuration; the dispatch shape in this ADR is unchanged. diff --git a/internal/scaffold/fullsend-repo/templates/shim-per-repo.yaml b/internal/scaffold/fullsend-repo/templates/shim-per-repo.yaml index d8c36fbda..0ce727435 100644 --- a/internal/scaffold/fullsend-repo/templates/shim-per-repo.yaml +++ b/internal/scaffold/fullsend-repo/templates/shim-per-repo.yaml @@ -13,6 +13,10 @@ # which determines the stage and conditionally calls the appropriate # reusable-{stage}.yml workflow. Adding a new stage requires only a case # branch in reusable-dispatch.yml — zero changes to this repo. +# +# Concurrency: per-role cancel-in-progress groups live in reusable-dispatch.yml +# stage jobs and agent-scoped groups on reusable-{stage}.yml — not on this shim. +# A monolithic shim group would serialize unrelated roles and drop pending runs (#2452). name: fullsend permissions: @@ -35,9 +39,6 @@ on: jobs: dispatch: - concurrency: - group: fullsend-dispatch-${{ github.event.issue.number || github.event.pull_request.number }} - cancel-in-progress: false if: >- github.event_name != 'issue_comment' || github.event.comment.user.type != 'Bot' diff --git a/internal/scaffold/scaffold_test.go b/internal/scaffold/scaffold_test.go index 0ca8f6c0d..6acaa564d 100644 --- a/internal/scaffold/scaffold_test.go +++ b/internal/scaffold/scaffold_test.go @@ -145,6 +145,21 @@ func TestShimWorkflowCallTemplateContent(t *testing.T) { assert.NotContains(t, s, "curl") } +func TestShimPerRepoTemplateContent(t *testing.T) { + content, err := FullsendRepoFile("templates/shim-per-repo.yaml") + require.NoError(t, err) + s := string(content) + assert.True(t, strings.HasPrefix(s, "---\n"), "per-repo shim must start with YAML document start marker") + assert.Contains(t, s, "dispatch:") + assert.Contains(t, s, "stop-fix:") + assert.Contains(t, s, "__REUSABLE_DISPATCH__") + assert.Contains(t, s, "install_mode: per-repo") + // Per-role concurrency lives in reusable-dispatch.yml, not a monolithic shim group (#2452). + assert.NotContains(t, s, "fullsend-dispatch-${{") + assert.NotContains(t, s, "concurrency:") + assert.Contains(t, s, "per-role cancel-in-progress groups live in reusable-dispatch.yml") +} + func TestShimTriggerParity(t *testing.T) { // Both shim templates must declare the same event trigger types so that // per-repo and workflow-call installation modes have identical behavior. diff --git a/internal/scaffold/workflow_call_alignment_test.go b/internal/scaffold/workflow_call_alignment_test.go index 0379396e7..756af0786 100644 --- a/internal/scaffold/workflow_call_alignment_test.go +++ b/internal/scaffold/workflow_call_alignment_test.go @@ -39,9 +39,107 @@ type callerWorkflow struct { } type callerJob struct { - Uses string `yaml:"uses"` - With map[string]string `yaml:"with"` - Secrets map[string]string `yaml:"secrets"` + Uses string `yaml:"uses"` + With map[string]string `yaml:"with"` + Secrets map[string]string `yaml:"secrets"` + Concurrency *jobConcurrency `yaml:"concurrency"` +} + +type jobConcurrency struct { + Group string `yaml:"group"` + CancelInProgress bool `yaml:"cancel-in-progress"` +} + +// reusableStageWorkflow includes workflow-level concurrency on reusable agent workflows. +type reusableStageWorkflow struct { + Concurrency *jobConcurrency `yaml:"concurrency"` + On reusableWorkflow `yaml:"on"` +} + +type stageConcurrencyExpectation struct { + groupPrefix string + groupMust []string +} + +var thinCallerConcurrencyExpectations = map[string]stageConcurrencyExpectation{ + "triage": { + groupPrefix: "fullsend-triage-", + groupMust: []string{"inputs.source_repo", "issue.number"}, + }, + "code": { + groupPrefix: "fullsend-code-", + groupMust: []string{"inputs.source_repo", "issue.number"}, + }, + "review": { + groupPrefix: "fullsend-review-", + groupMust: []string{"inputs.source_repo", "pull_request.number", "issue.number"}, + }, + "fix": { + groupPrefix: "fullsend-fix-", + groupMust: []string{"inputs.source_repo", "pull_request.number", "issue.number", "inputs.pr_number"}, + }, + "retro": { + groupPrefix: "fullsend-retro-", + groupMust: []string{"inputs.source_repo", "pull_request.number", "issue.number"}, + }, + "prioritize": { + groupPrefix: "fullsend-prioritize-", + groupMust: []string{"inputs.source_repo", "issue.number"}, + }, +} + +var reusableAgentConcurrencyExpectations = map[string]stageConcurrencyExpectation{ + "triage": { + groupPrefix: "fullsend-triage-agent-", + groupMust: []string{"inputs.source_repo", "issue.number"}, + }, + "code": { + groupPrefix: "fullsend-code-agent-", + groupMust: []string{"inputs.source_repo", "issue.number"}, + }, + "review": { + groupPrefix: "fullsend-review-agent-", + groupMust: []string{"inputs.source_repo", "pull_request.number", "issue.number"}, + }, + "fix": { + groupPrefix: "fullsend-fix-agent-", + groupMust: []string{"inputs.source_repo", "pull_request.number", "issue.number", "inputs.pr_number"}, + }, + "retro": { + groupPrefix: "fullsend-retro-agent-", + groupMust: []string{"inputs.source_repo", "pull_request.number", "issue.number"}, + }, + "prioritize": { + groupPrefix: "fullsend-prioritize-agent-", + groupMust: []string{"inputs.source_repo", "issue.number"}, + }, +} + +var dispatchStageConcurrencyExpectations = map[string]stageConcurrencyExpectation{ + "triage": { + groupPrefix: "fullsend-triage-", + groupMust: []string{"github.repository", "github.event.issue.number"}, + }, + "code": { + groupPrefix: "fullsend-code-", + groupMust: []string{"github.repository", "github.event.issue.number"}, + }, + "review": { + groupPrefix: "fullsend-review-", + groupMust: []string{"github.repository", "github.event.pull_request.number", "github.event.issue.number"}, + }, + "fix": { + groupPrefix: "fullsend-fix-", + groupMust: []string{"github.repository", "github.event.pull_request.number", "github.event.issue.number"}, + }, + "retro": { + groupPrefix: "fullsend-retro-", + groupMust: []string{"github.repository", "github.event.pull_request.number", "github.event.issue.number"}, + }, + "prioritize": { + groupPrefix: "fullsend-prioritize-", + groupMust: []string{"github.repository", "github.event.issue.number"}, + }, } // reusableWorkflowRef extracts the reusable workflow filename from a uses: reference. @@ -227,6 +325,83 @@ func TestReusableDispatchProjectNumberInput(t *testing.T) { "prioritize job should thread project_number from dispatch inputs") } +// TestReusableDispatchStageConcurrency validates per-role cancel-in-progress groups +// on all stage jobs in reusable-dispatch.yml (#981, #982, ADR 0033). +func TestReusableDispatchStageConcurrency(t *testing.T) { + content, err := os.ReadFile(filepath.Join("..", "..", ".github", "workflows", "reusable-dispatch.yml")) + require.NoError(t, err) + + var caller callerWorkflow + require.NoError(t, yaml.Unmarshal(content, &caller)) + + for stage, expect := range dispatchStageConcurrencyExpectations { + t.Run(stage, func(t *testing.T) { + job, ok := caller.Jobs[stage] + require.True(t, ok, "job %q should exist", stage) + require.NotNil(t, job.Concurrency, "job %q should declare a concurrency group", stage) + assert.Contains(t, job.Concurrency.Group, expect.groupPrefix) + for _, fragment := range expect.groupMust { + assert.Contains(t, job.Concurrency.Group, fragment, + "job %q concurrency group should reference %q", stage, fragment) + } + assert.True(t, job.Concurrency.CancelInProgress, + "job %q should cancel in-progress runs when a newer dispatch arrives", stage) + }) + } +} + +// TestReusableAgentWorkflowConcurrency validates agent-scoped cancel-in-progress +// groups on reusable stage workflows. Groups use a distinct -agent- prefix so +// they do not collide with dispatch/thin-caller groups on workflow_call parents. +func TestReusableAgentWorkflowConcurrency(t *testing.T) { + for stage, expect := range reusableAgentConcurrencyExpectations { + t.Run(stage, func(t *testing.T) { + path := filepath.Join("..", "..", ".github", "workflows", fmt.Sprintf("reusable-%s.yml", stage)) + content, err := os.ReadFile(path) + require.NoError(t, err) + + var wf reusableStageWorkflow + require.NoError(t, yaml.Unmarshal(content, &wf)) + require.NotNil(t, wf.Concurrency, "reusable-%s.yml should declare workflow-level concurrency", stage) + assert.Contains(t, wf.Concurrency.Group, expect.groupPrefix) + for _, fragment := range expect.groupMust { + assert.Contains(t, wf.Concurrency.Group, fragment, + "reusable-%s.yml concurrency group should reference %q", stage, fragment) + } + assert.True(t, wf.Concurrency.CancelInProgress, + "reusable-%s.yml should cancel in-progress runs", stage) + + callerExpect := thinCallerConcurrencyExpectations[stage] + assert.NotEqual(t, callerExpect.groupPrefix, expect.groupPrefix, + "reusable-%s.yml must use a distinct agent-scoped group prefix", stage) + assert.Contains(t, wf.Concurrency.Group, "-agent-", + "reusable-%s.yml group must be agent-scoped, not reuse dispatch/thin-caller prefix", stage) + }) + } +} + +// TestThinCallerStageConcurrency validates per-role cancel-in-progress groups on +// per-org thin caller workflows in the scaffold (#981, ADR 0033). +func TestThinCallerStageConcurrency(t *testing.T) { + for stage, expect := range thinCallerConcurrencyExpectations { + t.Run(stage, func(t *testing.T) { + path := fmt.Sprintf(".github/workflows/%s.yml", stage) + content := loadRenderedScaffoldCaller(path)(t) + + var wf reusableStageWorkflow + require.NoError(t, yaml.Unmarshal(content, &wf)) + require.NotNil(t, wf.Concurrency, "%s should declare workflow-level concurrency", path) + assert.Contains(t, wf.Concurrency.Group, expect.groupPrefix) + for _, fragment := range expect.groupMust { + assert.Contains(t, wf.Concurrency.Group, fragment, + "%s concurrency group should reference %q", path, fragment) + } + assert.True(t, wf.Concurrency.CancelInProgress, + "%s should cancel in-progress runs when a newer dispatch arrives", path) + }) + } +} + // TestReusableDispatchUsesFullyQualifiedPaths validates that reusable-dispatch.yml // references stage workflows with fully-qualified paths, not relative (./) paths. // Relative paths resolve against the caller's repo, which breaks per-repo mode