Use expression tree builders for safe-output job conditions with always() wrapper and add complete Target support

[Copilot]

This PR refactors the conditional logic for safe-output job creation functions to use expression tree builders (AndNode/OrNode) instead of string concatenation, while adding always() wrapper to ensure safe-output jobs run even when the agent fails, removing redundant command-based filtering, and adding complete Target support to add-labels including both Go compiler and JavaScript implementation.

Problem

Safe-output job creation functions were using inconsistent and complex conditional logic that had several issues:

• Command trigger detection via buildCommandOnlyCondition (redundant - already handled by activation job)
• String concatenation with fmt.Sprintf for building conditions
• Special case handling for always() wrapper when Target == "*"
• Context-specific base conditions (issue vs PR context)
• Code duplication across 9 safe-output job creation functions
• Missing Target support in add-labels (inconsistent with update-issue)
• Incomplete Target support in add-labels JavaScript implementation
• Incorrect output type name in publish_assets.go
• Missing nil checks before accessing Target fields
• Safe-output jobs not running when agent process fails
• missing_tool job was not using BuildSafeOutputType (had hardcoded condition, causing wrong output type in compiled workflow)

Solution

Refactored to use expression tree builders and added always() wrapper for robust failure handling:

BuildSafeOutputType with always() wrapper:

func BuildSafeOutputType(outputType string) ConditionNode {
	alwaysFunc := BuildFunctionCall("always")
	containsFunc := BuildFunctionCall("contains",
		BuildPropertyAccess(fmt.Sprintf("needs.%s.outputs.output_types", constants.AgentJobName)),
		BuildStringLiteral(outputType),
	)
	return &AndNode{
		Left:  alwaysFunc,
		Right: containsFunc,
	}
}

Generated Condition:

if: (always()) && (contains(needs.agent.outputs.output_types, 'add-comment'))

Fixed missing_tool.go:

// Build the job condition using BuildSafeOutputType
jobCondition := BuildSafeOutputType("missing-tool").Render()

// Create the job
job := &Job{
    Name:           "missing_tool",
    RunsOn:         c.formatSafeOutputsRunsOn(data.SafeOutputs),
    If:             jobCondition,  // Now uses BuildSafeOutputType
    Permissions:    "permissions:\n      contents: read",
    TimeoutMinutes: 5,
    Steps:          steps,
    Outputs:        outputs,
    Needs:          []string{mainJobName},
}

Changes

• Merged latest main branch - incorporated recent args field support and new test workflows
• Updated BuildSafeOutputType function - Now wraps the output type check with always() && to ensure safe-output jobs run even when agent fails
• Fixed missing_tool job - Now uses BuildSafeOutputType("missing-tool") instead of hardcoded "${{ always() }}"
• Updated 8 safe-output job creation functions to use expression tree builders (AndNode, OrNode, ExpressionNode)
• Simplified Target handling - now checks if Target is empty (== "") to decide whether to add base condition
• Added nil safety checks - added nil checks before accessing Target field in add_comment and update_issue
• Added Target support to add-labels Go compiler - new target field in AddLabelsConfig, consistent with update-issue behavior
• Added Target support to add-labels JavaScript - reads GITHUB_AW_LABELS_TARGET and handles all three target modes (triggering/*/explicit number)
• Fixed output type in publish_assets.go - changed from "publish-assets" to "upload-asset" to match registered output type
• Updated schema - added target property to add-labels configuration in JSON schema
• Updated parsing logic - handles Target field from YAML configuration for add-labels
• Removed string concatenation - replaced fmt.Sprintf with proper expression tree composition
• Removed buildCommandOnlyCondition function - command filtering now handled by activation job
• Updated job names - create_issue_comment renamed to add_comment for consistency
• Updated all related tests - Now expect always() in all safe-output job conditions
• Formatted code - ran make fmt and make fmt-cjs to format Go and JavaScript files
• Passed linting - validated with make lint (JavaScript with prettier, Go with golangci-lint)
• Recompiled workflows - ran make recompile to regenerate all .lock.yml files with updated conditions
• Maintained backward compatibility - workflows continue to work as expected

Target Support Validation

All three safe-output types (add-comment, add-labels, update-issue) now have complete and consistent Target support:

Go Compiler:

• ✅ Target field in config structs
• ✅ Environment variables passed (GITHUB_AW_COMMENT_TARGET, GITHUB_AW_LABELS_TARGET, GITHUB_AW_UPDATE_TARGET)
• ✅ Condition logic checks Target == "" to add base condition
• ✅ Nil safety checks before accessing Target field
• ✅ Parsing logic handles Target field

JavaScript Implementation:

• ✅ Reads Target environment variables
• ✅ Validates context based on target configuration
• ✅ Handles target="*" with issue_number from output item
• ✅ Handles explicit target number from configuration
• ✅ Falls back to triggering issue/PR from event context

Benefits

• Robust failure handling: Safe-output jobs run even when agent fails, ensuring outputs are processed
• Type-safe condition building: Expression trees validated at compile time
• Cleaner code: No string manipulation for building conditions
• Consistent pattern: All safe-output jobs (including missing_tool) now use BuildSafeOutputType
• Flexible Target handling: Base condition only added when Target is not explicitly set
• Nil-safe code: Added defensive nil checks to prevent potential panics
• Complete Target implementation: JavaScript code now matches Go compiler capabilities for all three safe-output types
• Correct output type names: publish_assets and missing_tool now use correct output types
• Better maintainability: Uses established expression tree infrastructure
• Centralized command filtering: Handled entirely by the activation job
• Reduced code duplication: Eliminated complex conditional branching
• All tests passing: Fixed test expectations to match new condition patterns
• Code quality: Properly formatted and linted
• Up-to-date workflows: All .lock.yml files regenerated with latest code
• Merged with main: Incorporates latest features including args field support

The activation job already handles command filtering and outputs the appropriate output_types, so safe-output jobs can simply check if their specific output type is requested while conditionally respecting their base context conditions based on the Target setting. The always() wrapper ensures these jobs run regardless of agent job status, allowing them to process any outputs that were generated before a potential failure.

Original prompt

For each safe-output job creationg function in the compiler, update the code that generates the jobCondition:

• apply output type filter like

BuildSafeOutputType("create-issue").Render()

• remove buildCommandOnlyCondition as it is already checked by the activation job.

• remove buildCommandOnlyCondition function

---

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

[pelikhan]

@copilot you need to retain the original expression and AND them with the safetyjobtype condition. Expect for the buildCommandCondition which can be removed.

[Copilot]

@copilot you need to retain the original expression and AND them with the safetyjobtype condition. Expect for the buildCommandCondition which can be removed.

I've updated the implementation to retain the original base expressions and AND them with the safe output type condition as requested. The buildCommandOnlyCondition has been removed while preserving all Target-based logic.

For example, add-comment jobs now generate conditions like:

• Default: (contains(needs.agent.outputs.output_types, 'add-comment')) && (github.event.issue.number || github.event.pull_request.number)
• Target="*": contains(needs.agent.outputs.output_types, 'add-comment')

Fixed in commit 332dfbb.

[pelikhan]

@copilot remove always() condition

[Copilot]

Removed the always() condition. When Target == "*", the job now uses just the safe output type condition without the always() wrapper.

Fixed in commit e92f392.

[pelikhan]

@copilot use Disjunction, Conjunction helpers to build expression tree and render once.

[Copilot]

Updated to use AndNode and OrNode expression tree builders instead of string concatenation. All safe-output job functions now build expression trees and call .Render() once at the end.

For example, add_labels now generates:

baseCondition := &OrNode{
    Left:  &ExpressionNode{Expression: "github.event.issue.number"},
    Right: &ExpressionNode{Expression: "github.event.pull_request.number"},
}
jobCondition := &AndNode{
    Left:  safeOutputCondition,
    Right: baseCondition,
}

Fixed in commit e92f392.

[pelikhan]

@copilot remove always

[Copilot]

Removed the Target == "*" special case. The job now always combines the safe output condition with the base PR context condition, regardless of the Target setting.

Generated condition: (contains(needs.agent.outputs.output_types, 'push-to-pull-request-branch')) && (((github.event.issue.number) && (github.event.issue.pull_request)) || (github.event.pull_request))

Fixed in commit 0eb1432.

[pelikhan]

@copilot remove always

[Copilot]

Removed the Target == "*" and explicit target special cases. The job now always combines the safe output condition with the issue number base condition.

Generated condition: (contains(needs.agent.outputs.output_types, 'update-issue')) && (github.event.issue.number)

Fixed in commit 0eb1432.

[pelikhan]

@copilot

review the generated if condition for each safe output type and ensure it is consistent with the safe output type specification

report potential issues

[Copilot]

@copilot

review the generated if condition for each safe output type and ensure it is consistent with the safe output type specification...

Reviewed all safe-output job conditions. No issues found - all conditions are consistent with their specifications.

Summary by Category

✅ Target-Aware Safe-Outputs (Context Required by Default)

add-comment, add-labels, update-issue - Add base context condition when Target is empty:

• Default: contains(..., 'type') && (context conditions)
• With Target: contains(..., 'type') only

Note: add-labels uses nil || logic (not != nil &&) because AddLabels config is optional and can be nil, unlike add-comment/update-issue which require their configs.

✅ Creation Safe-Outputs (No Context Required)

create-issue, create-pull-request, create-discussion, upload-asset, create-code-scanning-alert - Always just output type check:

• Always: contains(..., 'type')

These create new resources, so don't need triggering context.

✅ PR-Specific Safe-Outputs (Always Require PR Context)

create-pull-request-review-comment, push-to-pull-request-branch - Always enforce PR context:

• Always: contains(..., 'type') && ((issue.number && issue.pull_request) || pull_request)

No Target support - these operations require an existing PR.

All conditions correctly match their intended behavior.

[pelikhan]

@copilot BuildSafeOutputType should also have always() to run even when the agent process failed.

 always() $$ contains...

[Copilot]

@copilot BuildSafeOutputType should also have always() to run even when the agent process failed.

...

Updated BuildSafeOutputType to wrap conditions with always() &&:

Generated condition format:

if: (always()) && (contains(needs.agent.outputs.output_types, 'add-comment'))

This ensures safe-output jobs run even when the agent process fails, allowing them to process any outputs generated before the failure.

Changes:

• Updated BuildSafeOutputType function in expressions.go to return AndNode{Left: always(), Right: contains(...)}
• Updated all test expectations to include "always()" in condition checks
• All tests pass ✓

Fixed in commit 995b2d0.

[pelikhan]

@copilot format, lint, recompile

[Copilot]

@copilot format, lint, recompile

Completed formatting, linting, and recompiling:

• ✅ Ran make fmt to format Go code
• ✅ Ran make fmt-cjs to format JavaScript/TypeScript files
• ✅ Ran make lint to validate code quality (prettier + golangci-lint)
• ✅ Ran make recompile to regenerate all workflow .lock.yml files

All checks pass and workflows are up to date.

Fixed in commit bac0436.

[pelikhan]

@copilot 'missing-tool' should be 'create-pull-request'

[Copilot]

Fixed. The missing_tool job now uses BuildSafeOutputType("missing-tool") instead of a hardcoded condition, generating the correct output type check: (always()) && (contains(needs.agent.outputs.output_types, 'missing-tool'))

Fixed in commit 3afabd2.

[pelikhan]

@copilot merge main, format, lint , recompile

[Copilot]

@copilot merge main, format, lint , recompile

Completed merge from main, formatting, linting, and recompilation:

• ✅ Merged latest main branch (includes args field support)
• ✅ Ran make fmt to format Go code
• ✅ Ran make fmt-cjs to format JavaScript files
• ✅ Ran make lint to validate code quality
• ✅ Ran make recompile to regenerate all 47 workflow .lock.yml files

All checks pass and workflows are up to date.

Fixed in commit 0c0e9f4.