Reject minute units in stop-after relative time deltas by Copilot · Pull Request #2611 · github/gh-aw

Copilot · 2025-10-27T12:38:13Z

The stop-after field now rejects minute (m) units in relative time deltas. Minimum granularity is hours (h).

Changes

pkg/workflow/time_delta.go: Added parseTimeDeltaForStopAfter() that rejects minutes via new internal parseTimeDeltaWithMinutes(allowMinutes bool) function. Original parseTimeDelta() unchanged for log filtering compatibility.
pkg/workflow/stop_after.go: Switched from parseTimeDelta() to parseTimeDeltaForStopAfter()
Tests: New TestParseTimeDeltaForStopAfter() and TestResolveStopTimeRejectsMinutes(). Updated existing test cases using minutes in stop-after.
Documentation: Updated examples from +90m to +1d12h. Clarified minimum unit constraint.

Error Message

minute unit 'm' is not allowed for stop-after. Minimum unit is hours 'h'. Use +1h instead of +30m

Examples

# ✗ Invalid
stop-after: "+30m"
stop-after: "+1d12h30m"

# ✓ Valid  
stop-after: "+3h"
stop-after: "+1d12h"
stop-after: "+1mo2w3d12h"

Original prompt

refuse usage of minute, seconds relative time with stop-after. The "minimum" unit is hour.

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

github-actions · 2025-10-27T15:14:59Z

Agentic Changeset Generator triggered by this pull request.

Copilot

Pull Request Overview

This PR restricts the stop-after field to reject minute units in relative time deltas, enforcing hours as the minimum granularity. The change introduces a new parsing function specifically for stop-after while preserving backward compatibility for log filtering functionality.

Key Changes:

Added parseTimeDeltaForStopAfter() function that rejects minute units via internal parseTimeDeltaWithMinutes(allowMinutes bool) helper
Updated resolveStopTime() to use the new stricter parser
Added comprehensive test coverage for the minute rejection behavior

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
pkg/workflow/time_delta.go	Introduced `parseTimeDeltaForStopAfter()` and refactored parsing logic to support conditional minute validation
pkg/workflow/stop_after.go	Switched from `parseTimeDelta()` to `parseTimeDeltaForStopAfter()` for stop-after validation
pkg/workflow/time_delta_test.go	Added `TestParseTimeDeltaForStopAfter()` with minute rejection tests and updated existing test to expect errors
pkg/workflow/stop_after_test.go	Added `TestResolveStopTimeRejectsMinutes()` to verify end-to-end minute rejection
pkg/workflow/time_delta_integration_test.go	Updated integration tests to use valid time units and added minute rejection error cases
pkg/workflow/compiler_test.go	Changed example from `+30m` to `+3h`
pkg/cli/templates/instructions.md	Updated documentation to clarify minimum unit constraint and remove minute examples
docs/src/content/docs/reference/triggers.md	Updated documentation examples from `+90m` to valid formats
.github/instructions/github-agentic-workflows.instructions.md	Updated documentation to remove minute unit examples

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-10-27T15:15:39Z

pkg/workflow/time_delta_test.go

+					result.Minutes != tt.expected.Minutes || result.Weeks != tt.expected.Weeks ||
+					result.Months != tt.expected.Months {
+					t.Errorf("parseTimeDeltaForStopAfter(%q) = {Months: %d, Weeks: %d, Days: %d, Hours: %d, Minutes: %d}, want {Months: %d, Weeks: %d, Days: %d, Hours: %d, Minutes: %d}",
+						tt.input, result.Months, result.Weeks, result.Days, result.Hours, result.Minutes,
+						tt.expected.Months, tt.expected.Weeks, tt.expected.Days, tt.expected.Hours, tt.expected.Minutes)
+				}


The Minutes field check is unnecessary since parseTimeDeltaForStopAfter never allows minutes to be set. Consider removing the Minutes comparison from this assertion or explicitly verify it's always zero in valid cases.

Suggested change

result.Minutes != tt.expected.Minutes || result.Weeks != tt.expected.Weeks ||

result.Months != tt.expected.Months {

t.Errorf("parseTimeDeltaForStopAfter(%q) = {Months: %d, Weeks: %d, Days: %d, Hours: %d, Minutes: %d}, want {Months: %d, Weeks: %d, Days: %d, Hours: %d, Minutes: %d}",

tt.input, result.Months, result.Weeks, result.Days, result.Hours, result.Minutes,

tt.expected.Months, tt.expected.Weeks, tt.expected.Days, tt.expected.Hours, tt.expected.Minutes)

}

result.Weeks != tt.expected.Weeks || result.Months != tt.expected.Months {

t.Errorf("parseTimeDeltaForStopAfter(%q) = {Months: %d, Weeks: %d, Days: %d, Hours: %d}, want {Months: %d, Weeks: %d, Days: %d, Hours: %d}",

tt.input, result.Months, result.Weeks, result.Days, result.Hours,

tt.expected.Months, tt.expected.Weeks, tt.expected.Days, tt.expected.Hours)

}

if result.Minutes != 0 {

t.Errorf("parseTimeDeltaForStopAfter(%q) Minutes = %d, want 0", tt.input, result.Minutes)

}

Copilot · 2025-10-27T15:15:39Z

pkg/workflow/time_delta.go

 			delta.Hours = value
 		case "m":
+			if !allowMinutes {
+				return nil, fmt.Errorf("minute unit 'm' is not allowed for stop-after. Minimum unit is hours 'h'. Use +%dh instead of +%dm", (value+59)/60, value)


[nitpick] The calculation (value+59)/60 for rounding up minutes to hours could be replaced with a more explicit ceiling division for clarity. Consider using a helper function or a comment explaining the rounding behavior.

Initial plan

b66d64c

Copilot AI assigned Copilot and pelikhan Oct 27, 2025

Copilot started work on behalf of pelikhan October 27, 2025 12:38 View session

Copilot AI and others added 2 commits October 27, 2025 13:04

Refuse usage of minute units in stop-after relative time deltas

489610a

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Fix test helper function usage in stop_after_test.go

1946bd3

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Restrict time units to hours with stop-after~~ Reject minute units in stop-after relative time deltas Oct 27, 2025

Copilot AI requested a review from pelikhan October 27, 2025 13:11

Copilot finished work on behalf of pelikhan October 27, 2025 13:11

pelikhan approved these changes Oct 27, 2025

View reviewed changes

pelikhan marked this pull request as ready for review October 27, 2025 15:14

Copilot AI review requested due to automatic review settings October 27, 2025 15:14

Copilot AI reviewed Oct 27, 2025

View reviewed changes

pelikhan merged commit 57edcee into main Oct 27, 2025
7 checks passed

pelikhan deleted the copilot/refuse-minutes-seconds-usage branch October 27, 2025 15:16

github-actions bot mentioned this pull request Oct 28, 2025

[docs] Update documentation for features from 2025-10-28 #2659

Closed

Copilot AI mentioned this pull request Oct 28, 2025

[docs] Update documentation for features from 2025-10-28 #2670

Merged

github-actions bot mentioned this pull request Apr 2, 2026

[WHM] Workflow Health Dashboard — 2026-04-02 #24097

Closed

Copilot AI mentioned this pull request Apr 2, 2026

fix: silently skip add_comment when no triggering context on schedule runs #24131

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reject minute units in stop-after relative time deltas#2611

Reject minute units in stop-after relative time deltas#2611
pelikhan merged 3 commits intomainfrom
copilot/refuse-minutes-seconds-usage

Copilot AI commented Oct 27, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Oct 27, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Oct 27, 2025

Uh oh!

Copilot AI Oct 27, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Copilot AI commented Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Error Message

Examples

Uh oh!

github-actions bot commented Oct 27, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Oct 27, 2025 •

edited

Loading