refactor: use ExpressionBuilder for all if expressions in maintenance_workflow.go#26116
refactor: use ExpressionBuilder for all if expressions in maintenance_workflow.go#26116
Conversation
…_workflow.go Agent-Logs-Url: https://github.com/github/gh-aw/sessions/154f1ff2-da49-4ed7-95ab-ebf61d59523b Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/154f1ff2-da49-4ed7-95ab-ebf61d59523b Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot remove zizmor scan job |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/7d97d658-6f38-417c-b319-e34395beead9 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done — removed the Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Pull request overview
Refactors the maintenance workflow generator to consistently build all job-level if: expressions via the ExpressionBuilder API, and removes the zizmor-scan job from the generated maintenance workflow.
Changes:
- Replaced remaining raw string job
if:conditions inpkg/workflow/maintenance_workflow.gowithRenderCondition(...)using new helper builders. - Removed the
zizmor-scanjob from both the generator and the generated workflow YAML. - Updated the maintenance workflow test to stop expecting the removed job.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/maintenance_workflow.go | Uses ExpressionBuilder-based helpers for all job if: expressions; removes zizmor-scan job from generation. |
| pkg/workflow/maintenance_workflow_test.go | Updates operation-condition expectations to reflect removal of zizmor-scan. |
| .github/workflows/agentics-maintenance.yml | Regenerated workflow reflecting builder-rendered conditions and removal of zizmor-scan. |
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
| // Add compile-workflows job | ||
| yaml.WriteString(` | ||
| compile-workflows: | ||
| if: ${{ !github.event.repository.fork && (github.event_name != 'workflow_dispatch' || github.event.inputs.operation == '') }} | ||
| if: ${{ ` + RenderCondition(buildNotForkAndScheduled()) + ` }} |
There was a problem hiding this comment.
The comment above this dev-mode block still mentions adding a zizmor-scan job, but that job has been removed. Please update the comment to reflect the current jobs included in dev mode (e.g., compile-workflows / secret-validation) so future readers aren’t misled.
🧪 Test Quality Sentinel ReportTest Quality Score: 90/100✅ Excellent test quality
Test Classification Details
Change SummaryThe PR removes the Design test classification rationale: The test calls The test also handles edge cases: it checks for job-not-found ( No new tests for Score deduction (-10): The score is 90 rather than 100 because no new test coverage was added for the two new builder functions ( Language SupportTests analyzed:
Verdict
📖 Understanding Test ClassificationsDesign Tests (High Value) verify what the system does:
Implementation Tests (Low Value) verify how the system does it:
Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Test Quality Sentinel: 90/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). The single test modification correctly removes a stale reference to the deleted zizmor-scan job. No guideline violations detected.
Summary
Audits
pkg/workflow/maintenance_workflow.goand ensures every job-levelif:expression is built with theExpressionBuilderAPI rather than written as a raw string literal. Also removes thezizmor-scanjob from the maintenance workflow.Changes
New helper functions
buildNotForkAndScheduled()— builds!fork && (event_name != 'workflow_dispatch' || operation == "")for jobs that run on any non-dispatch event (schedule, push, etc.) or workflow_dispatch with empty operation.buildDispatchOperationCondition(operation string)— buildsdispatch && operation == op && !forkfor jobs triggered by a specific dispatch operation.Replaced hardcoded
if:expressions (6 jobs)close-expired-entitiesbuildNotForkAndScheduled()apply_safe_outputsbuildDispatchOperationCondition("safe_outputs")create_labelsbuildDispatchOperationCondition("create_labels")validate_workflowsbuildDispatchOperationCondition("validate")compile-workflowsbuildNotForkAndScheduled()secret-validationbuildNotForkAndScheduled()Removed job
zizmor-scanjob has been removed from the maintenance workflow generator.Updated generated file
agentics-maintenance.ymlregenerated viamake recompile— allif:conditions now use(!(github.event.repository.fork))form consistent with other ExpressionBuilder-generated jobs, and thezizmor-scanjob is no longer present.Testing
All existing
TestGenerateMaintenanceWorkflow_*tests pass. The tests usestrings.Containsto check for condition substrings, which remain present in the newly rendered expressions.