Skip to content

Commit c9f0894

Browse files
Copilotgh-aw-bot
andauthored
Wire typed OnStopAfter through production code; support runtime relative deltas
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
1 parent dcb8ec9 commit c9f0894

4 files changed

Lines changed: 167 additions & 10 deletions

File tree

‎actions/setup/js/check_stop_time.cjs‎

Lines changed: 73 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,64 @@
33

44
const { ERR_CONFIG, ERR_VALIDATION } = require("./error_codes.cjs");
55
const { writeDenialSummary } = require("./pre_activation_summary.cjs");
6+
7+
// Matches a relative time delta such as "+25h", "+3d", "+1w", "+1mo", "+1d12h".
8+
// Mirrors pkg/workflow/time_delta.go's parseTimeDeltaForStopAfter: minutes are not
9+
// supported since the minimum unit for stop-after is hours.
10+
const TIME_DELTA_PATTERN = /(\d+)(mo|w|d|h)/g;
11+
12+
/** @param {string} stopTime */
13+
function isRelativeStopTime(stopTime) {
14+
return stopTime.startsWith("+");
15+
}
16+
17+
/** @param {string} deltaStr */
18+
function parseTimeDeltaForStopAfter(deltaStr) {
19+
const rest = deltaStr.slice(1);
20+
if (!rest) {
21+
throw new Error("empty time delta after '+'");
22+
}
23+
24+
const matches = [...rest.matchAll(TIME_DELTA_PATTERN)];
25+
if (matches.length === 0) {
26+
throw new Error(`invalid time delta format: +${rest}. Expected format like +25h, +3d, +1w, +1mo, +1d12h`);
27+
}
28+
29+
const consumed = matches.reduce((sum, match) => sum + match[0].length, 0);
30+
if (consumed !== rest.length) {
31+
throw new Error(`invalid time delta format: +${rest}. Extra characters detected`);
32+
}
33+
34+
const delta = { months: 0, weeks: 0, days: 0, hours: 0 };
35+
const seenUnits = new Set();
36+
for (const [, valueStr, unit] of matches) {
37+
if (seenUnits.has(unit)) {
38+
throw new Error(`duplicate unit '${unit}' in time delta: +${rest}`);
39+
}
40+
seenUnits.add(unit);
41+
const value = parseInt(valueStr, 10);
42+
if (unit === "mo") delta.months = value;
43+
else if (unit === "w") delta.weeks = value;
44+
else if (unit === "d") delta.days = value;
45+
else if (unit === "h") delta.hours = value;
46+
}
47+
return delta;
48+
}
49+
50+
/**
51+
* Resolves a relative stop-time delta (e.g. "+48h") to an absolute Date, relative to baseTime.
52+
* @param {string} deltaStr
53+
* @param {Date} baseTime
54+
*/
55+
function resolveRelativeStopTime(deltaStr, baseTime) {
56+
const delta = parseTimeDeltaForStopAfter(deltaStr);
57+
const result = new Date(baseTime.getTime());
58+
result.setUTCMonth(result.getUTCMonth() + delta.months);
59+
result.setUTCDate(result.getUTCDate() + delta.weeks * 7 + delta.days);
60+
result.setUTCHours(result.getUTCHours() + delta.hours);
61+
return result;
62+
}
63+
664
async function main() {
765
const stopTime = process.env.GH_AW_STOP_TIME;
866
const workflowName = process.env.GH_AW_WORKFLOW_NAME;
@@ -19,8 +77,21 @@ async function main() {
1977

2078
core.info(`Checking stop-time limit: ${stopTime}`);
2179

22-
// Parse the stop time (format: "YYYY-MM-DD HH:MM:SS")
23-
const stopTimeDate = new Date(stopTime);
80+
// Resolve the stop time. A GitHub Actions expression (e.g. "${{ inputs.stop-after }}")
81+
// is passed through verbatim at compile time and evaluated by the runner before this
82+
// step runs, so it may still be a relative delta (e.g. "+48h") rather than an already
83+
// resolved absolute timestamp (format: "YYYY-MM-DD HH:MM:SS").
84+
let stopTimeDate;
85+
if (isRelativeStopTime(stopTime)) {
86+
try {
87+
stopTimeDate = resolveRelativeStopTime(stopTime, new Date());
88+
} catch (err) {
89+
core.setFailed(`${ERR_VALIDATION}: Invalid stop-time format: ${stopTime}. ${err instanceof Error ? err.message : String(err)}`);
90+
return;
91+
}
92+
} else {
93+
stopTimeDate = new Date(stopTime);
94+
}
2495

2596
if (Number.isNaN(stopTimeDate.getTime())) {
2697
core.setFailed(`${ERR_VALIDATION}: Invalid stop-time format: ${stopTime}. Expected format: YYYY-MM-DD HH:MM:SS`);

‎actions/setup/js/check_stop_time.test.cjs‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,5 +97,28 @@ const mockCore = {
9797
expect(mockCore.setOutput).toHaveBeenCalledWith("stop_time_ok", "false"),
9898
expect(mockCore.setFailed).not.toHaveBeenCalled());
9999
});
100+
}),
101+
describe("when stop time is a relative delta (e.g. resolved from a GitHub Actions expression)", () => {
102+
it("should allow execution for a future relative delta such as +48h", async () => {
103+
((process.env.GH_AW_STOP_TIME = "+48h"),
104+
(process.env.GH_AW_WORKFLOW_NAME = "test-workflow"),
105+
await eval(`(async () => { ${checkStopTimeScript}; await main(); })()`),
106+
expect(mockCore.setOutput).toHaveBeenCalledWith("stop_time_ok", "true"),
107+
expect(mockCore.setFailed).not.toHaveBeenCalled());
108+
});
109+
it("should support combined units such as +1d12h", async () => {
110+
((process.env.GH_AW_STOP_TIME = "+1d12h"),
111+
(process.env.GH_AW_WORKFLOW_NAME = "test-workflow"),
112+
await eval(`(async () => { ${checkStopTimeScript}; await main(); })()`),
113+
expect(mockCore.setOutput).toHaveBeenCalledWith("stop_time_ok", "true"),
114+
expect(mockCore.setFailed).not.toHaveBeenCalled());
115+
});
116+
it("should fail with a descriptive error for an invalid relative delta", async () => {
117+
((process.env.GH_AW_STOP_TIME = "+5x"),
118+
(process.env.GH_AW_WORKFLOW_NAME = "test-workflow"),
119+
await eval(`(async () => { ${checkStopTimeScript}; await main(); })()`),
120+
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("Invalid stop-time format")),
121+
expect(mockCore.setOutput).not.toHaveBeenCalled());
122+
});
100123
}));
101124
}));

‎pkg/workflow/stop_after.go‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,22 @@ func parseOnStopAfterValue(onMap map[string]any) (string, error) {
3939

4040
// extractStopAfterFromOn extracts the stop-after value from the on: section
4141
func (c *Compiler) extractStopAfterFromOn(frontmatter map[string]any, workflowData ...*WorkflowData) (string, error) {
42-
// Use cached On field from ParsedFrontmatter if available (when workflowData is provided)
43-
var onSection any
44-
var exists bool
45-
if len(workflowData) > 0 && workflowData[0] != nil && workflowData[0].ParsedFrontmatter != nil && workflowData[0].ParsedFrontmatter.On != nil {
46-
onSection = workflowData[0].ParsedFrontmatter.On
47-
exists = true
48-
} else {
49-
onSection, exists = frontmatter["on"]
42+
// Prefer the typed field populated by ParseFrontmatterConfig when available, so
43+
// production code has a single source of truth instead of reparsing the raw map.
44+
// ParseFrontmatterConfig silently leaves OnStopAfter empty on a parse error (e.g. a
45+
// non-string value), so fall back to re-parsing the raw on: map in that edge case to
46+
// surface the original compile error instead of silently treating it as unset.
47+
if len(workflowData) > 0 && workflowData[0] != nil && workflowData[0].ParsedFrontmatter != nil {
48+
pf := workflowData[0].ParsedFrontmatter
49+
if pf.OnStopAfter != "" {
50+
return pf.OnStopAfter, nil
51+
}
52+
return parseOnStopAfterValue(pf.On)
5053
}
5154

55+
// Fallback: no typed ParsedFrontmatter available, so parse the raw frontmatter map.
56+
onSection, exists := frontmatter["on"]
57+
5258
if !exists {
5359
return "", nil
5460
}

‎pkg/workflow/stop_after_test.go‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,63 @@ func TestExtractStopAfterFromOnMatchesTypedFieldValue(t *testing.T) {
383383
}
384384
}
385385

386+
// TestExtractStopAfterFromOnPrefersTypedFieldOverRawMap is a sentinel test proving that
387+
// extractStopAfterFromOn returns the typed OnStopAfter field instead of reparsing the raw
388+
// frontmatter map, by making the two diverge: the typed field is set explicitly to a
389+
// different value than on.stop-after in the raw map. If production code ever regresses to
390+
// reparsing the raw map instead of consuming the typed field, this test will fail.
391+
func TestExtractStopAfterFromOnPrefersTypedFieldOverRawMap(t *testing.T) {
392+
frontmatter := map[string]any{
393+
"on": map[string]any{
394+
"workflow_dispatch": nil,
395+
"stop-after": "+24h",
396+
},
397+
}
398+
399+
parsedFrontmatter := &FrontmatterConfig{
400+
On: frontmatter["on"].(map[string]any),
401+
OnStopAfter: "+999h", // deliberately diverges from the raw map's "+24h"
402+
}
403+
404+
compiler := NewCompiler()
405+
workflowData := &WorkflowData{ParsedFrontmatter: parsedFrontmatter}
406+
stopAfter, err := compiler.extractStopAfterFromOn(frontmatter, workflowData)
407+
if err != nil {
408+
t.Fatalf("extractStopAfterFromOn failed: %v", err)
409+
}
410+
if stopAfter != "+999h" {
411+
t.Errorf("extractStopAfterFromOn() = %q, want %q (the typed field value, not the raw map's %q)", stopAfter, "+999h", "+24h")
412+
}
413+
}
414+
415+
// TestExtractStopAfterFromOnSurfacesErrorWhenTypedFieldParseFailed verifies that when
416+
// ParseFrontmatterConfig leaves OnStopAfter empty because the raw value failed to parse
417+
// (e.g. a non-string value), extractStopAfterFromOn still surfaces the original parse
418+
// error instead of silently treating stop-after as unset.
419+
func TestExtractStopAfterFromOnSurfacesErrorWhenTypedFieldParseFailed(t *testing.T) {
420+
frontmatter := map[string]any{
421+
"on": map[string]any{
422+
"workflow_dispatch": nil,
423+
"stop-after": 123, // invalid: must be a string
424+
},
425+
}
426+
427+
parsedFrontmatter, err := ParseFrontmatterConfig(frontmatter)
428+
if err != nil {
429+
t.Fatalf("ParseFrontmatterConfig failed: %v", err)
430+
}
431+
if parsedFrontmatter.OnStopAfter != "" {
432+
t.Fatalf("Expected typed field to remain empty on parse failure, got %q", parsedFrontmatter.OnStopAfter)
433+
}
434+
435+
compiler := NewCompiler()
436+
workflowData := &WorkflowData{ParsedFrontmatter: parsedFrontmatter}
437+
_, err = compiler.extractStopAfterFromOn(frontmatter, workflowData)
438+
if err == nil {
439+
t.Fatal("Expected extractStopAfterFromOn to return an error for invalid stop-after type, got nil")
440+
}
441+
}
442+
386443
// TestProcessStopAfterConfigurationGitHubExpression verifies that a stop-after value
387444
// expressed as a GitHub Actions expression is passed through verbatim without being
388445
// parsed as a relative delta or absolute timestamp.

0 commit comments

Comments
 (0)