fix: merge imported workflow bots field in compiler

[Copilot]

When a workflow is imported, its bots field was extracted into MergedBots by import_field_extractor.go but never applied to the compiled output — imported bots were silently dropped.

Changes

• compiler_orchestrator_workflow.go: Replace bare extractBots call with mergeBots, mirroring the existing mergeSkipBots pattern:

  // Before
  workflowData.Bots = c.extractBots(frontmatter)
  
  // After
  workflowData.Bots = c.mergeBots(c.extractBots(frontmatter), importsResult.MergedBots)

• role_checks.go: Add mergeBots helper delegating to the shared mergeStringSlicesDedup utility (union with dedup, consistent with mergeSkipBots/mergeSkipRoles).

• bots_test.go: Add TestMergeBots covering top-only, imported-only, union, dedup, and nil cases. Add TestBotsImportMerge with three end-to-end sub-tests that compile a workflow importing another workflow with bots: defined, asserting the expected GH_AW_ALLOWED_BOTS value in the compiled lock file:

  • imported_bots_merged_with_top_level_bots — union of top-level on.bots and imported bots
  • imported_bots_only_no_top_level_bots — imported bots propagate when the main workflow defines none
  • duplicate_bots_across_top_level_and_import_deduped — overlapping bots are deduplicated

[Copilot]

Pull request overview

Fixes a compilation bug where on.bots values coming from imported workflows were collected into ImportsResult.MergedBots but not applied to the final compiled workflow, causing imported bots to be dropped.

Changes:

• Apply imported bots during compilation by merging extractBots(frontmatter) with importsResult.MergedBots.
• Add mergeBots helper (deduping union) alongside existing merge helpers.
• Add unit tests for mergeBots behavior (top-only, imported-only, union, dedup, nil).

Show a summary per file

File	Description
pkg/workflow/compiler_orchestrator_workflow.go	Merges extracted bots with imported bots during additional configuration extraction.
pkg/workflow/role_checks.go	Adds mergeBots helper delegating to shared dedup/merge utility.
pkg/workflow/bots_test.go	Adds TestMergeBots to validate merge/dedup behavior.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 3/3 changed files
• Comments generated: 1

[Copilot]

This change fixes imported bots propagation, but the added test only validates mergeBots in isolation. Consider adding an end-to-end test that compiles a workflow importing another workflow that defines on.bots, and assert the compiled lock output includes the expected GH_AW_ALLOWED_BOTS (and that top-level + imported bots are unioned/deduped). This will guard against regressions where the orchestrator stops applying importsResult.MergedBots.

[github-actions]

🧪 Test Quality Sentinel Report

Test Quality Score: 80/100

✅ Excellent test quality

Metric	Value
New/modified tests analyzed	1
✅ Design tests (behavioral contracts)	1 (100%)
⚠️ Implementation tests (low value)	0 (0%)
Tests with error/edge cases	1 (100%)
Duplicate test clusters	0
Test inflation detected	Yes (51 test lines vs 6 production lines, ratio 8.5:1)
🚨 Coding-guideline violations	None

---

Test Classification Details

Test	File	Classification	Issues Detected
TestMergeBots	pkg/workflow/bots_test.go	✅ Design	None — comprehensive table-driven test

---

Analysis

TestMergeBots (pkg/workflow/bots_test.go)

Classification: Design test — verifies the behavioral contract of mergeBots

Design invariant: The function must merge two bot lists, deduplicate entries, and handle nil inputs gracefully.

Table-driven cases (5 rows):

• top only — top list passed, imported nil
• imported only — top nil, imported list passed
• both with no overlap — distinct entries are merged
• both with duplicates deduped — overlapping entries appear exactly once ✅ (edge case)
• both nil — returns empty slice, not nil ✅ (boundary case)

What would break if deleted? A regression in deduplication logic (e.g., duplicated bots in the merged list) or incorrect nil handling would go undetected.

Assertion messages: Present ("mergeBots result mismatch") ✅

No mock libraries used ✅

---

Test Inflation Note

The test file gained 51 lines while production code gained 6 lines (ratio 8.5:1), triggering the inflation flag. However, this is intentional: TestMergeBots provides thorough table-driven coverage for a new helper function, which is the expected pattern in this codebase. The inflation is justified by the quality of the coverage.

---

Language Support

Tests analyzed:

• 🟨 JavaScript (*.test.cjs, *.test.js): 0 tests

---

Verdict

✅ Check passed. 0% of new tests are implementation tests (threshold: 30%). The TestMergeBots table-driven test provides genuine behavioral coverage including deduplication and nil-handling edge cases.

---

📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

• Assert on observable outputs, return values, or state changes
• Cover error paths and boundary conditions
• Would catch a behavioral regression if deleted
• Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

• Assert on internal function calls (mocking internals)
• Only test the happy path with typical inputs
• Break during legitimate refactoring even when behavior is correct
• Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.

References: §25120406059

🧪 Test quality analysis by Test Quality Sentinel · ● 518K · ◷

[github-actions]

✅ Test Quality Sentinel: 80/100. Test quality is acceptable — 0% of new tests are implementation tests (threshold: 30%).

[pelikhan]

@copilot review all comments

[Copilot]

@copilot review all comments

Added TestBotsImportMerge in bots_test.go (commit 46a1a37) with three end-to-end sub-tests that compile a workflow importing another workflow with bots: defined:

• imported_bots_merged_with_top_level_bots — union of top-level on.bots and imported bots
• imported_bots_only_no_top_level_bots — imported bots propagate when main workflow has none
• duplicate_bots_across_top_level_and_import_deduped — overlapping bots are deduplicated

All three assert the expected GH_AW_ALLOWED_BOTS value in the compiled lock file.

[github-actions]

✅ smoke-ci: safeoutputs CLI comment + comment-memory run (25123725952)

Generated by Smoke CI for issue #29151 · ◷

[github-actions]

Comment Memory

CI lights the path
Green checks bloom at dawn
Quiet bots still sing

Note

This comment is managed by comment memory.

It stores persistent context for this thread in the code block at the top of this comment.
Edit only the text inside the backtick fences; workflow metadata and the footer are regenerated automatically.

Learn more about comment memory

Generated by Smoke CI for issue #29151 · ◷