Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions .github/workflows/agentics-maintenance.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,17 @@
#
# For more information: https://github.github.com/gh-aw/introduction/overview/
#
# Alternative regeneration methods:
# make recompile
#
# Or use the gh-aw CLI directly:
# ./gh-aw compile --validate --verbose
#
# The workflow is generated when any workflow uses the 'expires' field
# in create-discussions, create-issues, or create-pull-request safe-outputs configuration.
# Schedule frequency is automatically determined by the shortest expiration time.
# This file defines the generated agentic maintenance workflow for this repository.
# It runs scheduled cleanup for expiring safe outputs and supports manual maintenance operations.
#
# This workflow is generated automatically when workflows use expiring safe outputs
# or when repository maintenance features are enabled in .github/workflows/aw.json.
#
# To disable maintenance workflow generation, set in .github/workflows/aw.json:
# {"maintenance": false}
#
Comment on lines +27 to +32
# Agentic maintenance docs:
# https://github.github.com/gh-aw/reference/ephemerals/#manual-maintenance-operations
#
name: Agentic Maintenance

Expand Down
4 changes: 4 additions & 0 deletions pkg/workflow/maintenance_workflow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,10 @@ func TestGenerateMaintenanceWorkflow_OperationJobConditions(t *testing.T) {
t.Fatalf("Expected maintenance workflow to be generated: %v", err)
}
yaml := string(content)
require.Contains(t, yaml, "This file defines the generated agentic maintenance workflow for this repository.")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/improve-codebase-architecture] Header contract assertions are appended to TestGenerateMaintenanceWorkflow_OperationJobConditions — a test whose name (and the lines that follow) are about job conditions, not header content. Future readers of this test won't expect header checks here, and a header wording change would surface as a confusingly-named test failure.

💡 Suggestion: dedicated header test

Extract these four assertions (and any future header assertions) into a new function:

func TestGenerateMaintenanceWorkflow_HeaderContent(t *testing.T) {
    t.Parallel()
    content, err := buildMaintenanceWorkflowYAML(/* minimal args */)
    require.NoError(t, err)
    yaml := string(content)

    require.Contains(t, yaml, "This file defines the generated agentic maintenance workflow for this repository.")
    require.Contains(t, yaml, "This workflow is generated automatically when workflows use expiring safe outputs")
    require.Contains(t, yaml, `{"maintenance": false}`)
    require.Contains(t, yaml, "https://github.github.com/gh-aw/reference/ephemerals/#manual-maintenance-operations")
}

Keeping the header contract in its own test makes the intent clear and isolates header failures from job-condition failures.

require.Contains(t, yaml, "This workflow is generated automatically when workflows use expiring safe outputs")
require.Contains(t, yaml, `{"maintenance": false}`)
require.Contains(t, yaml, "https://github.github.com/gh-aw/reference/ephemerals/#manual-maintenance-operations")
Comment on lines +419 to +422

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Header assertions bolted to an unrelated test: these four require.Contains checks verify header content but live inside TestGenerateMaintenanceWorkflow_OperationJobConditions, which is scoped to operation job conditions.

💡 Suggested fix

Move the assertions into a dedicated test, either a new TestGenerateMaintenanceWorkflow_HeaderContent or a short test near TestGenerateMaintenanceWorkflow_WithExpires. That way, when the header text changes and CI fails, the test name immediately identifies it as a header regression rather than a job-conditions failure.

func TestGenerateMaintenanceWorkflow_HeaderContent(t *testing.T) {
    // minimal generation call, then:
    require.Contains(t, yaml, "This file defines the generated agentic maintenance workflow")
    require.Contains(t, yaml, "This workflow is generated automatically when workflows use expiring safe outputs")
    require.Contains(t, yaml, `{"maintenance": false}`)
    require.Contains(t, yaml, "https://github.github.com/gh-aw/reference/ephemerals/#manual-maintenance-operations")
}

The current placement is not a correctness bug — the assertions do protect the right contract — but poor test naming will slow down diagnosis when the header changes again.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] The test asserts that new strings are present, but doesn't verify that the old generic header content is absent. If the generator were to accidentally emit both the old and new headers, all four assertions would still pass.

💡 Suggestion: add a negative assertion

Add one require.NotContains to lock out the old content:

require.NotContains(t, yaml, "Alternative regeneration methods", "old generic header should not appear")

This makes the test a true before/after contract rather than only a presence check.


operationSkipCondition := `github.event_name != 'workflow_dispatch' && github.event_name != 'workflow_call' || inputs.operation == ''`
operationRunCondition := `(github.event_name == 'workflow_dispatch' || github.event_name == 'workflow_call') && inputs.operation != '' && inputs.operation != 'safe_outputs' && inputs.operation != 'create_labels' && inputs.operation != 'activity_report' && inputs.operation != 'close_agentic_workflows_issues' && inputs.operation != 'clean_cache_memories' && inputs.operation != 'update_pull_request_branches' && inputs.operation != 'validate' && inputs.operation != 'forecast'`
Expand Down
16 changes: 9 additions & 7 deletions pkg/workflow/maintenance_workflow_yaml.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,17 @@ func buildMaintenanceWorkflowYAML(
var yaml strings.Builder

// Add workflow header with logo and instructions
customInstructions := `Alternative regeneration methods:
make recompile
customInstructions := `This file defines the generated agentic maintenance workflow for this repository.
It runs scheduled cleanup for expiring safe outputs and supports manual maintenance operations.

Or use the gh-aw CLI directly:
./gh-aw compile --validate --verbose
This workflow is generated automatically when workflows use expiring safe outputs
or when repository maintenance features are enabled in .github/workflows/aw.json.

The workflow is generated when any workflow uses the 'expires' field
in create-discussions, create-issues, or create-pull-request safe-outputs configuration.
Schedule frequency is automatically determined by the shortest expiration time.`
To disable maintenance workflow generation, set in .github/workflows/aw.json:
{"maintenance": false}
Comment on lines +60 to +64

Agentic maintenance docs:
https://github.github.com/gh-aw/reference/ephemerals/#manual-maintenance-operations`

header := GenerateWorkflowHeader("", "pkg/workflow/maintenance_workflow.go", customInstructions)
yaml.WriteString(header)
Expand Down
Loading