Skip to content

fix: conflict detection for triggers/pipelines in MergeApplicationConfig, async context propagation, comment accuracy#145

Merged
intel352 merged 2 commits intofeat/issue-80-multi-workflowfrom
copilot/sub-pr-133
Feb 23, 2026
Merged

fix: conflict detection for triggers/pipelines in MergeApplicationConfig, async context propagation, comment accuracy#145
intel352 merged 2 commits intofeat/issue-80-multi-workflowfrom
copilot/sub-pr-133

Conversation

Copy link
Contributor

Copilot AI commented Feb 23, 2026

MergeApplicationConfig silently overwrote duplicate trigger and pipeline names across merged workflow files, and ConfigDir was inconsistently set from the first workflow file rather than the application config. The async step.workflow_call goroutine used context.Background(), decoupling it entirely from parent cancellation. A misleading comment in the server startup path misrepresented the actual build flow.

Changes

  • Conflict detection (config/config.go): MergeApplicationConfig now tracks seen trigger and pipeline names (via seenTriggers/seenPipelines) and returns a descriptive error on collision — matching the existing module conflict behavior:

    application "my-app": trigger name conflict: trigger "http-in" is defined in both "api" and "events"
    application "my-app": pipeline name conflict: pipeline "process-order" is defined in both "orders" and "fulfillment"
    
  • ConfigDir consistency (config/config.go): combined.ConfigDir is now pre-set from appCfg.ConfigDir before the merge loop; the per-workflow fallback only applies when the application config dir is unset.

  • Async context propagation (module/pipeline_step_workflow_call.go): The fire-and-forget goroutine now derives its timeout context from the parent ctx instead of context.Background(), allowing cancellation signals to propagate:

    go func(parentCtx context.Context, data map[string]any) {
        asyncCtx, cancel := context.WithTimeout(parentCtx, s.timeout)
        defer cancel()
        _, _ = target.Execute(asyncCtx, data)
    }(ctx, triggerData)
  • Comment fix (cmd/server/main.go): Removed the incorrect reference to BuildFromApplicationConfig; comment now accurately describes the MergeApplicationConfig → buildEngine → BuildFromConfig flow.

  • Tests (engine_multi_config_test.go): Added TestBuildFromApplicationConfig_TriggerNameConflict and TestBuildFromApplicationConfig_PipelineNameConflict.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

…, clarify comment

Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Copilot AI changed the title [WIP] Add multi-workflow application config and cross-workflow invocation fix: conflict detection for triggers/pipelines in MergeApplicationConfig, async context propagation, comment accuracy Feb 23, 2026
Copilot AI requested a review from intel352 February 23, 2026 14:51
@intel352 intel352 marked this pull request as ready for review February 23, 2026 16:23
@intel352 intel352 merged commit 0e1f61a into feat/issue-80-multi-workflow Feb 23, 2026
@intel352 intel352 deleted the copilot/sub-pr-133 branch February 23, 2026 16:23
intel352 added a commit that referenced this pull request Feb 23, 2026
…tion (#133)

* feat: add multi-workflow application config and cross-workflow invocation (#80)

- Add ApplicationConfig format for multi-workflow applications referencing
  separate workflow YAML files with shared module registry
- Add step.workflow_call pipeline step for cross-workflow invocation with
  sync (call-and-wait) and async (fire-and-forget) modes
- Add input/output mapping with template expansion for data passing
- Add pipelineRegistry to StdEngine for runtime pipeline lookup
- Add BuildFromApplicationConfig() with module name conflict detection
- Add auto-detection of application configs in cmd/server
- Add step.workflow_call schema to module schemas
- Add example multi-workflow app (chat platform with 3 workflows)
- Add comprehensive tests for workflow call step and multi-config loading

Closes #80

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: conflict detection for triggers/pipelines in MergeApplicationConfig, async context propagation, comment accuracy (#145)

* Initial plan

* fix: add conflict detection for triggers/pipelines, fix async context, clarify comment

Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
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.

2 participants