Skip to content

Commit 8699e63

Browse files
Copilotpelikhan
andauthored
fix: harden copilot steering state load and save
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/10b67751-6800-491f-97fd-2b102bbee5ca Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
1 parent 044efbf commit 8699e63

2 files changed

Lines changed: 53 additions & 2 deletions

File tree

actions/setup/js/copilot_steering_hook.cjs

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,15 @@
33
"use strict";
44

55
const fs = require("fs");
6+
const path = require("path");
67

78
const DEFAULT_TIMEOUT_MINUTES = 30;
89
const DEFAULT_TIME_WARNING_MINUTES = 5;
910
const DEFAULT_TIME_CRITICAL_MINUTES = 2;
1011
const DEFAULT_RUN_WARNING_REMAINING = 2;
1112
const DEFAULT_RUN_CRITICAL_REMAINING = 1;
1213
const DEFAULT_STATE_PATH = "/tmp/gh-aw/copilot-steering-state.json";
14+
const MS_PER_MINUTE = 60000;
1315

1416
/**
1517
* @typedef {{
@@ -77,7 +79,11 @@ function loadState(statePath) {
7779
return null;
7880
}
7981
const raw = fs.readFileSync(statePath, "utf8");
80-
return /** @type {SteeringState} */ JSON.parse(raw);
82+
const parsed = JSON.parse(raw);
83+
if (!isValidSteeringState(parsed)) {
84+
return null;
85+
}
86+
return parsed;
8187
} catch {
8288
return null;
8389
}
@@ -88,9 +94,29 @@ function loadState(statePath) {
8894
* @param {SteeringState} state
8995
*/
9096
function saveState(statePath, state) {
97+
fs.mkdirSync(path.dirname(statePath), { recursive: true });
9198
fs.writeFileSync(statePath, JSON.stringify(state), "utf8");
9299
}
93100

101+
/**
102+
* @param {unknown} value
103+
* @returns {value is SteeringState}
104+
*/
105+
function isValidSteeringState(value) {
106+
if (!value || typeof value !== "object") {
107+
return false;
108+
}
109+
const candidate = /** @type {Record<string, unknown>} */ value;
110+
return (
111+
typeof candidate.startedAtMs === "number" &&
112+
Number.isFinite(candidate.startedAtMs) &&
113+
typeof candidate.turns === "number" &&
114+
Number.isFinite(candidate.turns) &&
115+
typeof candidate.warningInjected === "boolean" &&
116+
typeof candidate.criticalInjected === "boolean"
117+
);
118+
}
119+
94120
/**
95121
* @param {unknown} value
96122
* @returns {number}
@@ -140,7 +166,7 @@ function buildBudgetSummary(remainingMinutes, remainingRuns) {
140166
*/
141167
function computeSteeringDecision(state, config, timestamp) {
142168
const nextState = { ...state, turns: state.turns + 1 };
143-
const elapsedMinutes = (timestamp - nextState.startedAtMs) / 60000;
169+
const elapsedMinutes = (timestamp - nextState.startedAtMs) / MS_PER_MINUTE;
144170
const remainingMinutes = Number.isFinite(config.timeoutMinutes) ? config.timeoutMinutes - elapsedMinutes : null;
145171
const remainingRuns = config.maxRuns > 0 ? config.maxRuns - nextState.turns : null;
146172

actions/setup/js/copilot_steering_hook.test.cjs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,4 +80,29 @@ describe("copilot_steering_hook.cjs", () => {
8080
expect(result.decision.reason).toContain("CRITICAL");
8181
expect(result.decision.reason).toContain("minute(s) left");
8282
});
83+
84+
it("falls back to initial state when persisted state is malformed-but-parseable", () => {
85+
const env = makeTestEnv({
86+
GH_AW_TIMEOUT_MINUTES: "10",
87+
GH_AW_STEERING_TIME_WARNING_MINUTES: "9.9",
88+
GH_AW_STEERING_TIME_CRITICAL_MINUTES: "0.1",
89+
GH_AW_COPILOT_MAX_RUNS: "2",
90+
});
91+
fs.writeFileSync(statePath, "{}", "utf8");
92+
const result = handleSteeringEvent("agentStop", { timestamp: 60 * 1000 }, env);
93+
expect(result.state.turns).toBe(1);
94+
expect(result.state.warningInjected).toBe(true);
95+
expect(result.decision).not.toBeNull();
96+
expect(result.decision.reason).toContain("minute(s) left");
97+
});
98+
99+
it("creates parent directory before saving state", () => {
100+
const env = makeTestEnv();
101+
const nestedStatePath = path.join(tempDir, "nested", "deeper", "state.json");
102+
env.GH_AW_COPILOT_STEERING_STATE_PATH = nestedStatePath;
103+
104+
const result = handleSteeringEvent("sessionStart", { timestamp: 1000, source: "new" }, env);
105+
expect(result.decision).toBeNull();
106+
expect(fs.existsSync(nestedStatePath)).toBe(true);
107+
});
83108
});

0 commit comments

Comments
 (0)