Skip to content

Commit 014ef77

Browse files
fix(lanes): actions:read scope, terminal error envelope, first-due grace (#1414, #1430)
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
1 parent 2423479 commit 014ef77

5 files changed

Lines changed: 163 additions & 15 deletions

File tree

‎.github/workflows/scheduled-lane-health.yml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ on:
1818

1919
permissions:
2020
contents: read
21+
# Reading workflow run history is what this job is: an explicit map makes every omitted scope
22+
# `none`, so leaving `actions` out would 403 the collection step.
23+
actions: read
2124
issues: write
2225

2326
concurrency:

‎scripts/scheduled-lane/github-api.ts‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ type RunsResponse = {
1111
workflow_runs?: { conclusion: string | null; created_at: string; html_url: string }[];
1212
};
1313

14+
type WorkflowResponse = { created_at?: string };
15+
1416
type IssuesResponse = { number: number; title: string }[];
1517

1618
function headers(token: string): Record<string, string> {
@@ -58,6 +60,19 @@ function toLaneRun(run: { conclusion: string | null; created_at: string; html_ur
5860
};
5961
}
6062

63+
/** When the lane was added: it decides whether an empty history is a newborn lane or a dead one. */
64+
async function fetchRegisteredAt(
65+
token: string,
66+
repo: string,
67+
workflow: string,
68+
): Promise<string | null> {
69+
const body = await requestOrNullOn404<WorkflowResponse>(
70+
token,
71+
`${API}/repos/${repo}/actions/workflows/${workflow}`,
72+
);
73+
return body?.created_at ?? null;
74+
}
75+
6176
/**
6277
* Newest-first scheduled runs. A 404 means the workflow is not on the default branch yet (a lane
6378
* being born, as in this PR), which is unknown history rather than a dark lane.
@@ -73,7 +88,11 @@ export async function fetchScheduledRuns(input: {
7388
if (body === null) {
7489
return { known: false, reason: 'workflow not on the default branch yet', runs: [] };
7590
}
76-
return { known: true, runs: (body.workflow_runs ?? []).map(toLaneRun) };
91+
return {
92+
known: true,
93+
registeredAt: await fetchRegisteredAt(input.token, input.repo, input.workflow),
94+
runs: (body.workflow_runs ?? []).map(toLaneRun),
95+
};
7796
}
7897

7998
/** Creates the alert issue, or comments on it when it is already open. */

‎scripts/scheduled-lane/health-model.test.ts‎

Lines changed: 60 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
// Every category comes from recorded run fields, so these cases pin the exact boundaries an alert
44
// fires on: a lane that stopped running, one that failed twice, and one that is merely flaky.
55

6+
import { spawnSync } from 'node:child_process';
7+
import fs from 'node:fs';
8+
import os from 'node:os';
69
import path from 'node:path';
710
import { fileURLToPath } from 'node:url';
811
import { describe, expect, it } from 'vitest';
@@ -25,8 +28,12 @@ const NIGHTLY: LaneCadence = {
2528
cadenceHours: 24,
2629
};
2730

28-
function known(runs: LaneRun[]) {
29-
return { known: true, runs };
31+
function known(runs: LaneRun[], registeredHoursAgo = 1000) {
32+
return {
33+
known: true,
34+
registeredAt: new Date(NOW - registeredHoursAgo * 60 * 60 * 1000).toISOString(),
35+
runs,
36+
};
3037
}
3138

3239
function run(hoursAgo: number, conclusion: LaneRun['conclusion']): LaneRun {
@@ -75,13 +82,25 @@ describe('lane health', () => {
7582
expect(lane.reason).toContain('over 48h of cadence');
7683
});
7784

78-
it('is dark when the lane has never run on schedule', () => {
85+
it('is dark when a long-registered lane has never run on schedule', () => {
7986
const lane = laneHealth(NIGHTLY, known([]), NOW);
8087
expect(lane.state).toBe('dark');
8188
expect(lane.lastRunAt).toBeNull();
8289
expect(lane.hoursSinceLastSuccess).toBeNull();
8390
});
8491

92+
it('gives a newborn lane the same two-cadence grace before its first run', () => {
93+
const lane = laneHealth(NIGHTLY, known([], 5), NOW);
94+
expect(lane.state).toBe('pending');
95+
expect(lane.reason).toContain('first run not yet 48h overdue');
96+
expect(unhealthyLanes([lane])).toEqual([]);
97+
});
98+
99+
it('stays pending when a run-less lane has no registration date to judge', () => {
100+
const lane = laneHealth(NIGHTLY, { known: true, registeredAt: null, runs: [] }, NOW);
101+
expect(lane.state).toBe('pending');
102+
});
103+
85104
it('is pending, not dark, when the run history could not be read', () => {
86105
const lane = laneHealth(NIGHTLY, { known: false, reason: 'no token', runs: [] }, NOW);
87106
expect(lane.state).toBe('pending');
@@ -94,6 +113,44 @@ describe('lane health', () => {
94113
});
95114
});
96115

116+
describe('terminal failures', () => {
117+
// A monitor whose API call fails must still leave evidence behind, or the lane it watches goes
118+
// dark twice over: once in CI and once in the artifact a human would look for.
119+
it('writes an error envelope and snapshot when the Actions API is unreachable', () => {
120+
const artifactDir = fs.mkdtempSync(path.join(os.tmpdir(), 'lane-health-'));
121+
const result = spawnSync(
122+
process.execPath,
123+
[
124+
'--experimental-strip-types',
125+
path.join(REPO_ROOT, 'scripts/scheduled-lane/health.ts'),
126+
'--artifact-dir',
127+
artifactDir,
128+
'--dry-run',
129+
],
130+
{
131+
cwd: REPO_ROOT,
132+
encoding: 'utf8',
133+
env: {
134+
...process.env,
135+
GITHUB_TOKEN: 'not-a-real-token',
136+
GITHUB_API_URL: 'http://127.0.0.1:1',
137+
},
138+
},
139+
);
140+
expect(result.status).toBe(1);
141+
const envelope = JSON.parse(
142+
fs.readFileSync(path.join(artifactDir, 'run-envelope.json'), 'utf8'),
143+
);
144+
expect(envelope.result).toBe('error');
145+
expect(envelope.lane).toBe('scheduled-lane-health');
146+
expect(String(envelope.details.error).length).toBeGreaterThan(0);
147+
const snapshot = JSON.parse(
148+
fs.readFileSync(path.join(artifactDir, 'lane-health.json'), 'utf8'),
149+
);
150+
expect(snapshot.error).not.toBeNull();
151+
});
152+
});
153+
97154
describe('lane discovery', () => {
98155
// Derived from the workflow directory: a scheduled lane cannot opt out of being watched.
99156
it('finds every schedule-triggered workflow in this repo', () => {

‎scripts/scheduled-lane/health-model.ts‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ export type LaneHistory = {
2626
/** False when the run history could not be read at all, which is not evidence of a dark lane. */
2727
known: boolean;
2828
reason?: string;
29+
/** When the lane was registered, so a newborn lane is not judged before its first run is due. */
30+
registeredAt?: string | null;
2931
/** Newest-first, as the GitHub runs API returns them. */
3032
runs: readonly LaneRun[];
3133
};
@@ -82,14 +84,38 @@ function countLeadingFailures(runs: readonly LaneRun[]): number {
8284
return failed === -1 ? runs.length : failed;
8385
}
8486

87+
/**
88+
* A lane with no runs yet is only dark once it has existed long enough to have missed the same
89+
* number of cadences as a lane that stopped: born-yesterday lanes get the identical grace.
90+
*/
91+
function classifyNeverRan(
92+
budget: number,
93+
ageHours: number | null,
94+
): { state: LaneState; reason: string } {
95+
if (ageHours === null) {
96+
return { state: 'pending', reason: 'no scheduled run yet and registration date unknown' };
97+
}
98+
if (ageHours <= budget) {
99+
return {
100+
state: 'pending',
101+
reason: `added ${ageHours.toFixed(1)}h ago, first run not yet ${budget}h overdue`,
102+
};
103+
}
104+
return {
105+
state: 'dark',
106+
reason: `no scheduled run in the ${ageHours.toFixed(1)}h since the lane was added`,
107+
};
108+
}
109+
85110
function classify(input: {
86111
runs: readonly LaneRun[];
87112
cadenceHours: number;
88113
hoursSinceLastRun: number | null;
114+
ageHours: number | null;
89115
consecutiveFailures: number;
90116
}): { state: LaneState; reason: string } {
91117
const budget = input.cadenceHours * CADENCES_BEFORE_ALERT;
92-
if (input.runs.length === 0) return { state: 'dark', reason: 'no scheduled run recorded yet' };
118+
if (input.runs.length === 0) return classifyNeverRan(budget, input.ageHours);
93119
if (input.hoursSinceLastRun !== null && input.hoursSinceLastRun > budget) {
94120
return {
95121
state: 'dark',
@@ -118,6 +144,7 @@ export function laneHealth(cadence: LaneCadence, history: LaneHistory, now: numb
118144
runs,
119145
cadenceHours: cadence.cadenceHours,
120146
hoursSinceLastRun,
147+
ageHours: hoursSince(history.registeredAt ?? null, now),
121148
consecutiveFailures,
122149
})
123150
: {

‎scripts/scheduled-lane/health.ts‎

Lines changed: 52 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -83,33 +83,75 @@ async function alert(options: Options, unhealthy: readonly LaneHealth[]): Promis
8383
process.stdout.write(`Alert issue #${result.number} ${result.action}.\n`);
8484
}
8585

86-
async function main(): Promise<number> {
87-
const startedAt = Date.now();
88-
const options = readOptions(process.argv.slice(2));
89-
const lanes = await collectHealth(options);
90-
const unhealthy = unhealthyLanes(lanes);
86+
function writeSnapshot(
87+
options: Options,
88+
startedAt: number,
89+
lanes: readonly LaneHealth[],
90+
error?: string,
91+
): void {
9192
fs.mkdirSync(options.artifactDir, { recursive: true });
93+
const snapshot = { generatedAt: new Date(startedAt).toISOString(), lanes, error: error ?? null };
9294
fs.writeFileSync(
9395
path.join(options.artifactDir, SNAPSHOT_FILE),
94-
`${JSON.stringify({ generatedAt: new Date(startedAt).toISOString(), lanes }, null, 2)}\n`,
96+
`${JSON.stringify(snapshot, null, 2)}\n`,
9597
);
96-
writeSummary(lanes, unhealthy);
97-
await alert(options, unhealthy);
98+
}
99+
100+
function writeEnvelope(
101+
options: Options,
102+
startedAt: number,
103+
result: 'pass' | 'fail' | 'error',
104+
details: Record<string, unknown>,
105+
): void {
98106
writeLaneEnvelope(
99107
options.artifactDir,
100108
buildLaneEnvelope({
101109
lane: 'scheduled-lane-health',
102110
tool: 'scripts/scheduled-lane/health.ts',
103-
result: unhealthy.length === 0 ? 'pass' : 'fail',
111+
result,
104112
startedAt,
105113
finishedAt: Date.now(),
106114
config: { repo: options.repo, dryRun: options.dryRun, runsPerLane: RUNS_PER_LANE },
107-
details: { lanes, unhealthy: unhealthy.map((lane) => lane.workflow) },
115+
details,
108116
}),
109117
);
118+
}
119+
120+
async function run(options: Options, startedAt: number): Promise<number> {
121+
const lanes = await collectHealth(options);
122+
const unhealthy = unhealthyLanes(lanes);
123+
writeSnapshot(options, startedAt, lanes);
124+
writeSummary(lanes, unhealthy);
125+
await alert(options, unhealthy);
126+
writeEnvelope(options, startedAt, unhealthy.length === 0 ? 'pass' : 'fail', {
127+
lanes,
128+
unhealthy: unhealthy.map((lane) => lane.workflow),
129+
});
110130
// Dark/failing lanes are reported, not enforced by this job's own status: the alert issue is the
111131
// signal, and a red health job would itself be a lane that needs watching.
112132
return 0;
113133
}
114134

135+
/**
136+
* A monitor that dies silently is worse than no monitor: an API/permission failure still has to
137+
* leave a snapshot and an `error` envelope behind, and fail loudly (unlike an unhealthy lane).
138+
*/
139+
function reportTerminalFailure(options: Options, startedAt: number, error: unknown): number {
140+
const message = error instanceof Error ? error.message : String(error);
141+
writeSnapshot(options, startedAt, [], message);
142+
writeEnvelope(options, startedAt, 'error', { lanes: [], unhealthy: [], error: message });
143+
process.stderr.write(`Scheduled lane health failed: ${message}\n`);
144+
return 1;
145+
}
146+
147+
async function main(): Promise<number> {
148+
const startedAt = Date.now();
149+
const options = readOptions(process.argv.slice(2));
150+
try {
151+
return await run(options, startedAt);
152+
} catch (error) {
153+
return reportTerminalFailure(options, startedAt, error);
154+
}
155+
}
156+
115157
process.exitCode = await main();

0 commit comments

Comments
 (0)