feat: add auto_upgrade_cron to aw.json for configurable auto-upgrade schedule#46521
Conversation
…schedule - Add auto_upgrade_cron field to repo_config_schema.json with cron pattern validation - Add AutoUpgradeCron field to RepoConfig struct, wire through GenerateAutoUpdateWorkflow - Add CustomCron option to GenerateAutoUpdateWorkflowOptions; used verbatim when set - Add autoUpgradeCronFrom helper and pass custom cron at all three call sites in maintenance_workflow.go - Set auto_upgrade_cron: 0 9 * * 1 in .github/workflows/aw.json (Monday 09:00 UTC) - Regenerate agentic-auto-upgrade.yml with the explicit cron schedule - Add unit tests for custom cron override and aw.json parsing Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
|
Hey
|
|
@copilot make it a child field of auto upgrade in object form auto_upgrade: { cron: ... } |
…ade object Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in commit {
"auto_upgrade": { "cron": "0 9 * * 1" }
}The boolean forms ( |
🤖 PR Triage
Automated triage — run §29677420780
|
There was a problem hiding this comment.
Pull request overview
Adds configurable auto-upgrade scheduling through auto_upgrade.cron.
Changes:
- Extends repository configuration and workflow generation with custom cron support.
- Propagates custom schedules through maintenance generation paths.
- Adds tests and regenerates this repository’s auto-upgrade workflow.
Show a summary per file
| File | Description |
|---|---|
pkg/workflow/repo_config.go |
Parses custom auto-upgrade configuration. |
pkg/workflow/repo_config_test.go |
Tests configuration loading and validation. |
pkg/workflow/maintenance_workflow.go |
Propagates custom cron values. |
pkg/workflow/auto_update_workflow.go |
Generates workflows using custom schedules. |
pkg/workflow/auto_update_workflow_test.go |
Tests custom schedule generation. |
pkg/parser/schemas/repo_config_schema.json |
Defines the new configuration shape. |
.github/workflows/aw.json |
Configures Monday 09:00 UTC upgrades. |
.github/workflows/agentic-auto-upgrade.yml |
Applies the configured schedule. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 8/8 changed files
- Comments generated: 3
- Review effort level: Medium
| "description": "Custom cron expression for the agentic-auto-upgrade workflow schedule. When set, overrides the default fuzzy weekly schedule. Must be a valid 5-field POSIX cron expression (e.g. '0 9 * * 1' for Monday at 09:00 UTC).", | ||
| "type": "string", | ||
| "minLength": 9, | ||
| "pattern": "^[0-9*/,\\- ]+ [0-9*/,\\- ]+ [0-9*/,\\- ]+ [0-9*/,\\- ]+ [0-9*/,\\- ]+$", |
| "auto_upgrade": { | ||
| "description": "When true, the compiler generates agentic-auto-upgrade.yml that runs on a fuzzy weekly schedule and inlines the 'upgrade' operation to check for and report available workflow upgrades via a GitHub issue. Defaults to false (opt-in).", | ||
| "type": "boolean" | ||
| "description": "Controls agentic-auto-upgrade.yml generation. Set to true to enable with the default fuzzy weekly schedule, false to disable, or an object to configure a custom schedule. Defaults to false (opt-in).", | ||
| "oneOf": [ |
| if opts.CustomCron != "" { | ||
| cronSchedule = opts.CustomCron | ||
| autoUpdateWorkflowLog.Printf("Using custom cron schedule: %q", cronSchedule) |
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. |
|
✅ Test Quality Sentinel completed test quality analysis. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review
The implementation is well-structured with good test coverage, but three blocking issues in the existing inline comments must be resolved before merge:
-
Weak cron regex (
repo_config_schema.jsonline 53) — the pattern allows spaces within each field's character class, so values like99 99 99 99 99and six-field expressions pass validation. The regex must be tightened. -
Schema/implementation shape mismatch (line 39) — the PR description references a top-level
auto_upgrade_cronfield alongside a booleanauto_upgrade, but the implementation uses a nestedauto_upgrade.cronobject. The schema and documentation should match the actual shape. -
Stale generated-workflow comment (
auto_update_workflow.goline 93) — whenCustomCronis set, the generated YAML template still carries the "fuzzy weekly" comment. It should reflect the custom schedule instead.
Please address the three existing inline comments before merging.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · 16.1 AIC · ⌖ 4.38 AIC · ⊞ 5K
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (161 new lines in 📄 Draft ADR committed:
📋 What to do next
Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision. ❓ Why ADRs Matter
ADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors (and your future self) will thank you. 📋 Michael Nygard ADR Format ReferenceAn ADR must contain these four sections to be considered complete:
All ADRs are stored in
|
🧪 Test Quality Sentinel Report✅ Test Quality Score: 90/100 — Excellent
📊 Metrics (6 tests)
i️ Test Detailsauto_update_workflow_test.go (2 tests, 38 lines added, 16 lines prod code)
repo_config_test.go (1 table-driven test with 4 sub-tests, 43 lines added, 31 lines prod code)
Verdict
Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /codebase-design, /grill-with-docs, and /tdd — requesting changes on three issues (one new, two pre-existing from prior review).
📋 Key Themes & Highlights
Key Themes
- Stale generated-file comment:
buildAutoUpdateWorkflowYAMLalways emits "The weekly schedule is deterministically scattered based on the repository slug" even when a custom cron is used — this will actively mislead maintainers. - Unenforced invariant:
AutoUpgradeCrondoc says it requiresAutoUpgradeto be true, butautoUpgradeCronFromdoes not callIsAutoUpgradeEnabled, so a hand-crafted struct can silently pass a cron to a disabled upgrade workflow. - Schema cron regex (raised in prior review): the pattern passes any five whitespace-separated tokens (e.g.
99 99 99 99 99), so schema validation provides weaker protection than it implies.
Positive Highlights
- ✅ Polymorphic JSON parsing via
json.RawMessageis consistent with the existingmaintenancefield pattern. - ✅ Good test coverage: happy path, empty-object form, boolean form, and schema rejection of invalid cron.
- ✅ Helper
autoUpgradeCronFromkeeps all three call sites clean and nil-safe.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 36.8 AIC · ⌖ 4.64 AIC · ⊞ 6.7K
Comment /matt to run again
Comments that could not be inline-anchored
pkg/workflow/auto_update_workflow.go:173
[/grill-with-docs] The customInstructions string embedded in the generated workflow hardcodes "The weekly schedule is deterministically scattered based on the repository slug." — this is factually incorrect when a custom cron is active and will confuse maintainers who inspect the generated file.
<details>
<summary>💡 Suggested fix</summary>
Make the instruction conditional, or use a more neutral description:
scheduleNote := "The weekly schedule is deterministically scattered base…
</details>
<details><summary>pkg/workflow/repo_config.go:173</summary>
**[/codebase-design]** The `AutoUpgradeCron` field has a doc comment saying "Requires AutoUpgrade to be true", but this invariant is not enforced anywhere — it is silently ignored if someone constructs a `RepoConfig` with a non-nil `AutoUpgrade` pointing to `false` but a non-empty `AutoUpgradeCron`. The `IsAutoUpgradeEnabled` method or `autoUpgradeCronFrom` helper could guard against this inconsistency.
<details>
<summary>💡 Suggested fix</summary>
In `autoUpgradeCronFrom`:
```go
func autoUpg…
</details>
There was a problem hiding this comment.
REQUEST_CHANGES — one high, one medium issue
High: CustomCron is string-concatenated into the YAML template without sanitization at the generation layer. The JSON schema guard only fires through LoadRepoConfig; direct callers can inject arbitrary YAML characters (newline, backslash, quote).
Medium: Empty-cron object form (auto_upgrade: {}) silently falls back to FUZZY:WEEKLY — this is surprising and undocumented in both the struct comment and autoUpgradeCronFrom.
Previously raised (existing open comments)
- The cron regex allows spaces inside each field character class, so values like
99 99 99 99 99or 6-field expressions pass schema validation. - The generated workflow YAML header still claims the schedule is "deterministically scattered" when a custom cron is in use.
- PR description shows a top-level
auto_upgrade_cronkey but the actual implementation uses nestedauto_upgrade.cron— the description is misleading.
🔎 Code quality review by PR Code Quality Reviewer · 55.4 AIC · ⌖ 4.64 AIC · ⊞ 5.6K
Comment /review to run again
Comments that could not be inline-anchored
pkg/workflow/auto_update_workflow.go:181
YAML injection risk: cronSchedule is concatenated directly into the YAML template without sanitization, and callers that set CustomCron directly on GenerateAutoUpdateWorkflowOptions bypass the JSON schema guard entirely.
<details>
<summary>💡 Details and suggested fix</summary>
The generated YAML line is built as:
- cron: "" + cronSchedule + "" // Weekly (auto-upgrade)A value like "0 9 * * 1\\nname: injected" breaks out of the quoted scalar and injects arbitrary YAML.…
pkg/workflow/repo_config.go:240
Silent fallback when object form has no cron: auto_upgrade: {} or auto_upgrade: {"cron": ""} silently falls back to FUZZY:WEEKLY with no user-visible indication, but the struct comment says "Requires AutoUpgrade to be true" without documenting this implicit behavior.
<details>
<summary>💡 Details</summary>
When the object form is used without a cron key, AutoUpgradeCron is set to "" and the caller in maintenance_workflow.go passes an empty string to `GenerateAutoUpdateWork…
|
@copilot run pr-finisher skill |
|
@copilot please run the Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "pi.dev"See Network Configuration for more information.
|
…custom schedules - Schema: fix cron pattern to reject spaces inside fields (prevents 6-field values like "0 0 * * * *" from matching) - Go: add validateCronExpression with per-field range checks (rejects "99 99 99 99 99") - auto_update_workflow.go: make header and schedule comment conditional on custom vs fuzzy cron - Regenerate agentic-auto-upgrade.yml with correct custom schedule comments - Tests: add coverage for out-of-range and 6-field rejections; assert correct comment text in custom cron tests Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done —
Done in commit
|
…eration comment Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
All blockers have been addressed:
Also fixed a pre-existing
|
The auto-upgrade workflow schedule was previously only configurable via fuzzy weekly scattering (deterministic per repo slug). This adds an explicit
auto_upgrade_cronfield toaw.jsonfor repos that need a predictable, fixed schedule.Schema & config
repo_config_schema.json— newauto_upgrade_cronstring property with 5-field POSIX cron pattern validationRepoConfig— newAutoUpgradeCron stringfield, parsed and trimmed inUnmarshalJSONGeneration
GenerateAutoUpdateWorkflowOptions.CustomCron— when non-empty, used verbatim; skipsFUZZY:WEEKLYscattermaintenance_workflow.go— all threeGenerateAutoUpdateWorkflowcall sites passCustomCronvia newautoUpgradeCronFrom(cfg)helperThis repo
{ "auto_upgrade": true, "auto_upgrade_cron": "0 9 * * 1" }Pins the schedule to Monday 09:00 UTC;
agentic-auto-upgrade.ymlregenerated accordingly.Tests
TestGenerateAutoUpdateWorkflow_CustomCron/TestGenerateAutoUpdateWorkflow_CustomCronOverridesFuzzy— verifies custom cron is used verbatim and overrides per-repo scatterTestLoadRepoConfig_AutoUpgradeCron— load, omit, and invalid pattern cases (rejection via JSON schema validation)Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
pi.devSee Network Configuration for more information.