Conversation
…ows job in maintenance workflow generator Agent-Logs-Url: https://github.com/github/gh-aw/sessions/520bb413-72d8-43b4-8d6f-9bcc967cedf2 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…solidate test constant Agent-Logs-Url: https://github.com/github/gh-aw/sessions/520bb413-72d8-43b4-8d6f-9bcc967cedf2 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…rigger Agent-Logs-Url: https://github.com/github/gh-aw/sessions/5320a8c7-8f36-45b4-a81f-431583ec8fe6 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in commit 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:
|
.github/workflows/*.md to agentic maintenance workflow generator.github/workflows/*.md to agentic maintenance workflow generator
|
@copilot set group concurrency to cancel if a new build arrives for the compile workflows job |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/6efcea04-672a-4501-a877-6c14041a8c30 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in commit concurrency:
group: ${{ github.workflow }}-compile-workflows-${{ github.repository }}
cancel-in-progress: trueWarning 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
Updates the Agentic Maintenance workflow generator to add a dev-mode push trigger (scoped to default-branch changes to .github/workflows/*.md) and to ensure only the compile-workflows job runs for those push events, with concurrency configured to cancel redundant runs.
Changes:
- Add dev-mode
pushtrigger on the repo default branch for.github/workflows/*.mdchanges. - Update job
if:conditions so schedule-only jobs exclude push events whilecompile-workflowsstill runs on push. - Add
compile-workflowsjob concurrency and thread repository slug/default-branch resolution through the generator + tests.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/side_repo_maintenance_integration_test.go | Updates generator invocation to pass new repoSlug parameter. |
| pkg/workflow/maintenance_workflow_yaml.go | Emits dev-mode push trigger, updates job conditions, adds compile-workflows concurrency. |
| pkg/workflow/maintenance_workflow_test.go | Adds/updates tests for push trigger behavior, job conditions, and concurrency. |
| pkg/workflow/maintenance_workflow.go | Adds default-branch resolution via gh api and threads it into YAML generation. |
| pkg/workflow/maintenance_conditions.go | Introduces push-excluding condition builders for schedule-only jobs. |
| pkg/workflow/compiler_types.go | Exposes repository slug via GetRepositorySlug(). |
| pkg/cli/init.go | Passes compiler repository slug into maintenance workflow generation. |
| pkg/cli/compile_post_processing.go | Passes compiler repository slug into maintenance workflow generation. |
| .github/workflows/agentics-maintenance.yml | Regenerated workflow including push trigger and compile-workflows concurrency. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
pkg/workflow/maintenance_workflow_test.go:651
- Same out-of-bounds slicing risk here:
jobSection := yaml[compileIdx : compileIdx+jobSectionSearchRange]can panic if the YAML is shorter than expected. Bound the slice end withmin(len(yaml), compileIdx+jobSectionSearchRange)to keep the test robust.
jobSection := yaml[compileIdx : compileIdx+jobSectionSearchRange]
if strings.Contains(jobSection, "github.event_name != 'push'") {
t.Errorf("Job compile-workflows should NOT exclude push events, but condition is:\n%s", jobSection)
}
- Files reviewed: 9/9 changed files
- Comments generated: 4
| // Fetch the default branch for the push trigger (dev mode only) | ||
| // Resolved here to avoid passing it through multiple layers; empty slug falls back to "main" | ||
| defaultBranch := FetchDefaultBranch(repoSlug) |
There was a problem hiding this comment.
FetchDefaultBranch(repoSlug) is called unconditionally, so release-mode generation will still invoke gh api (or at least attempt to) even though the push trigger is only emitted in dev mode. This adds unnecessary network/auth dependency and can slow or flake non-dev runs. Consider only resolving the default branch when actionMode == ActionModeDev (or when the push trigger will be included), and otherwise pass an empty/default value through to buildMaintenanceWorkflowYAML.
| // Fetch the default branch for the push trigger (dev mode only) | |
| // Resolved here to avoid passing it through multiple layers; empty slug falls back to "main" | |
| defaultBranch := FetchDefaultBranch(repoSlug) | |
| // Fetch the default branch only when the dev-mode push trigger will be included. | |
| // Outside dev mode, leave it empty so generation does not depend on gh/api auth or network access. | |
| defaultBranch := "" | |
| if actionMode == ActionModeDev { | |
| defaultBranch = FetchDefaultBranch(repoSlug) | |
| } |
| yaml.WriteString(` push: | ||
| branches: | ||
| - ` + defaultBranch + ` | ||
| paths: | ||
| - '.github/workflows/*.md' |
There was a problem hiding this comment.
defaultBranch is interpolated into YAML without quoting/escaping. Unusual branch names (e.g. containing : or starting with YAML 1.1 boolean keywords like on/yes/no) or unexpected characters from the API response could produce invalid YAML or surprising parsing. Safer options: quote the branch value (e.g. single-quote with escaping) and/or validate it against an allowed branch-name pattern before writing, falling back to the default when invalid.
| jobSection := yaml[jobIdx : jobIdx+jobSectionSearchRange] | ||
| if !strings.Contains(jobSection, pushExclusionCondition) { | ||
| t.Errorf("Job %q should exclude push events (%q) but condition is:\n%s", job, pushExclusionCondition, jobSection) | ||
| } |
There was a problem hiding this comment.
The test slices jobSection := yaml[jobIdx : jobIdx+jobSectionSearchRange] without checking bounds; if the generated YAML is shorter than jobIdx+jobSectionSearchRange this will panic and fail the test for an unrelated reason. Use end := min(len(yaml), jobIdx+jobSectionSearchRange) (similar to the earlier min(500, len(yaml)) usage) before slicing.
This issue also appears on line 648 of the same file.
| // non-dispatch/call event including push, or on workflow_dispatch/workflow_call | ||
| // with an empty operation, and never on forks. Unlike buildNotForkAndScheduleOnly, | ||
| // this function does NOT exclude push events. | ||
| // Condition: !fork && ((event_name != 'workflow_dispatch' && event_name != 'workflow_call') || operation == ”) |
There was a problem hiding this comment.
This condition comment uses a typographic closing quote (”) rather than an ASCII quote, which makes the example hard to read/copy and is inconsistent with the rest of the file. Consider changing it to operation == '' (or operation == "") to match the actual expression being built.
| // Condition: !fork && ((event_name != 'workflow_dispatch' && event_name != 'workflow_call') || operation == ”) | |
| // Condition: !fork && ((event_name != 'workflow_dispatch' && event_name != 'workflow_call') || operation == '') |
🧪 Test Quality Sentinel ReportTest Quality Score: 93/100✅ Excellent test quality
Test Classification DetailsView all 6 analyzed tests
Verdict
The new Minor observation (non-blocking): One subtest ( Build tags are correct: 📖 Understanding Test ClassificationsDesign Tests (High Value) verify what the system does:
Implementation Tests (Low Value) verify how the system does it:
References: §24899268915
|
There was a problem hiding this comment.
✅ Test Quality Sentinel: 93/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). The new push-trigger tests are well-structured behavioral contracts covering dev mode, release mode, branch substitution, and per-job push exclusion guards.
Summary
Updates the Agentic Maintenance workflow generator so the generated
agentics-maintenance.ymlalso triggers on pushes to the repository's default branch that affect.github/workflows/*.mdfiles. When this push event fires, only thecompile-workflowsjob runs — all other scheduled jobs are excluded. Thecompile-workflowsjob also uses a concurrency group withcancel-in-progress: trueso redundant builds are cancelled when a new push arrives.Changes
Generator changes (
pkg/workflow/)maintenance_conditions.go:buildNotForkAndScheduleOnly()— same asbuildNotForkAndScheduled()but adds an explicitgithub.event_name != 'push'guardbuildNotForkAndScheduleOnlyOrOperation()— same as the removedbuildNotForkAndScheduledOrOperation()but also excludes push eventsbuildNotForkAndScheduledOrOperation()(all callers migrated)buildNotForkAndScheduled()comment to make clear it intentionally includes push eventsmaintenance_workflow.go:FetchDefaultBranch(slug string) string— queriesgh api /repos/{slug} --jq .default_branchat compilation time to determine the repository's default branch, falling back to"main"when the slug is empty or the API call failsrepoSlug stringparameter toGenerateMaintenanceWorkflowto thread the repository slug through to branch resolutioncompiler_types.go:GetRepositorySlug()getter onCompilerto expose the already-resolved repository slugmaintenance_workflow_yaml.go:pushtrigger (dev mode only, sincecompile-workflowsis dev-mode only) using the API-resolved default branch:concurrencygroup tocompile-workflowsjob withcancel-in-progress: true:close-expired-entities,cleanup-cache-memory, andsecret-validationjob conditions to use the new push-excluding builderscompile-workflowscondition is unchanged — it usesbuildNotForkAndScheduled()which includes push eventscompile_post_processing.goandinit.go:compiler.GetRepositorySlug()toGenerateMaintenanceWorkflowso the default branch can be resolved at compile timemaintenance_workflow_test.go:TestGenerateMaintenanceWorkflow_PushTriggerwith sub-tests verifying push trigger presence in dev mode, absence in release mode, push exclusion on schedule-only jobs, push inclusion forcompile-workflows, concurrency group, and correct substitution of a custom branch nameGenerated file
.github/workflows/agentics-maintenance.yml: Regenerated viamake recompileGenerated output (agentics-maintenance.yml)