-
Notifications
You must be signed in to change notification settings - Fork 425
feat: add copilot CLI steering hooks for time and run budgets #30500
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 7 commits
adcb563
8d19d75
506da7b
f8bdd5d
c8dcc9b
5a0f35f
044efbf
8699e63
9a583dd
ef60b87
62cc12e
8c6cbdf
9f2fd5a
378cd5b
557e0e5
23aa210
4c62720
55b570d
fd8060a
cf52407
3597233
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -67,6 +67,9 @@ const MAX_SCHEDULED_EXIT2_RETRIES = 1; | |
| // If prompt files are larger than this threshold, avoid inlining into argv. | ||
| const PROMPT_FILE_INLINE_THRESHOLD_BYTES = 100 * 1024; | ||
| const PROMPT_FILE_INLINE_THRESHOLD_LABEL = "100KB"; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oog! Me see DEFAULT_MAX_AUTOPILOT_RUNS = 1. Maybe add comment explain why 1 is good default? Cave brain want know. |
||
| const STEERING_HOOK_CONFIG_FILENAME = "gh-aw-steering.json"; | ||
| const DEFAULT_STEERING_STATE_PATH = "/tmp/gh-aw/copilot-steering-state.json"; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Consider using a more descriptive constant name, e.g., |
||
| const DEFAULT_MAX_AUTOPILOT_RUNS = 1; | ||
|
|
||
| // Pattern to detect transient CAPIError 400 in copilot output | ||
| const CAPI_ERROR_400_PATTERN = /CAPIError:\s*400/; | ||
|
|
@@ -295,6 +298,123 @@ function resolvePromptFileArgs(args) { | |
| return resolvedArgs; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Me see steering hook install. Good idea! Hook run before agent start. But me wonder: what happen if hook script path wrong? Error logged but agent still run — maybe worth adding metrics counter here too. |
||
| } | ||
|
|
||
| /** | ||
| * Parse --max-autopilot-continues from copilot CLI args. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ugh! parseMaxAutopilotContinues good function. Me think return 0 when not found - maybe return DEFAULT_MAX_AUTOPILOT_RUNS instead for consistent behavior? Just caveman thought. |
||
| * @param {string[]} args | ||
| * @returns {number} | ||
| */ | ||
| function parseMaxAutopilotContinues(args) { | ||
| const index = args.indexOf("--max-autopilot-continues"); | ||
| if (index < 0 || index + 1 >= args.length) { | ||
| return 0; | ||
| } | ||
| const parsed = parseInt(args[index + 1], 10); | ||
| return Number.isFinite(parsed) && parsed > 0 ? parsed : 0; | ||
| } | ||
|
|
||
| /** | ||
| * Compute the maximum number of autonomous runs (initial run + autopilot continuations). | ||
| * @param {string[]} args | ||
| * @returns {number} | ||
| */ | ||
| function computeMaxAutopilotRuns(args) { | ||
| if (!args.includes("--autopilot")) { | ||
| return DEFAULT_MAX_AUTOPILOT_RUNS; | ||
| } | ||
| const maxContinues = parseMaxAutopilotContinues(args); | ||
| if (maxContinues <= 0) { | ||
| return DEFAULT_MAX_AUTOPILOT_RUNS; | ||
| } | ||
| return maxContinues + 1; | ||
| } | ||
|
|
||
| /** | ||
| * Build Copilot CLI hook config for gh-aw steering messages. | ||
| * @param {string} hookScriptPath | ||
| * @param {string} nodeExecPath | ||
| * @returns {{ version: number, hooks: Record<string, Array<{ type: string, bash: string, timeoutSec: number }>> }} | ||
| */ | ||
| function buildSteeringHookConfig(hookScriptPath, nodeExecPath) { | ||
| // JSON-encode paths so they are safely quoted when embedded into bash hook command strings. | ||
| const quotedNodePath = JSON.stringify(nodeExecPath); | ||
| const quotedHookScriptPath = JSON.stringify(hookScriptPath); | ||
| return { | ||
| version: 1, | ||
| hooks: { | ||
| sessionStart: [ | ||
| { | ||
| type: "command", | ||
| bash: `${quotedNodePath} ${quotedHookScriptPath} sessionStart`, | ||
| timeoutSec: 10, | ||
| }, | ||
| ], | ||
| agentStop: [ | ||
| { | ||
| type: "command", | ||
| bash: `${quotedNodePath} ${quotedHookScriptPath} agentStop`, | ||
| timeoutSec: 10, | ||
| }, | ||
| ], | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Install Copilot CLI steering hooks in the workspace and export hook env vars. | ||
| * @param {string[]} resolvedArgs | ||
| */ | ||
| function installCopilotSteeringHooks(resolvedArgs) { | ||
| try { | ||
| const workspace = process.env.GITHUB_WORKSPACE || process.cwd(); | ||
| const hooksDir = path.join(workspace, ".github", "hooks"); | ||
| const hookConfigPath = path.join(hooksDir, STEERING_HOOK_CONFIG_FILENAME); | ||
| const hookScriptPath = path.join(__dirname, "copilot_steering_hook.cjs"); | ||
|
|
||
| if (!fs.existsSync(hookScriptPath)) { | ||
| log(`warning: steering hook script missing at ${hookScriptPath}; this may indicate setup action copy/deploy drift, so Copilot steering hooks will be skipped`); | ||
| return; | ||
| } | ||
|
|
||
| // Include run ID, PID, and timestamp to avoid collisions across concurrent/serial runner jobs. | ||
| // This installer runs once per harness process, so exactly one state path is expected per run. | ||
| const runID = process.env.GITHUB_RUN_ID || "local"; | ||
| const stateDir = path.join(path.dirname(DEFAULT_STEERING_STATE_PATH), "steering-hooks"); | ||
| fs.mkdirSync(stateDir, { recursive: true }); | ||
| const processStatePath = path.join(stateDir, `copilot-steering-${runID}-${process.pid}-${Date.now()}.json`); | ||
| process.env.GH_AW_COPILOT_STEERING_STATE_PATH = processStatePath; | ||
| process.env.GH_AW_COPILOT_MAX_RUNS = String(computeMaxAutopilotRuns(resolvedArgs)); | ||
| process.env.GH_AW_TIMEOUT_MINUTES = process.env.GH_AW_TIMEOUT_MINUTES || "30"; | ||
| process.env.GH_AW_STEERING_TIME_WARNING_MINUTES = process.env.GH_AW_STEERING_TIME_WARNING_MINUTES || "5"; | ||
| process.env.GH_AW_STEERING_TIME_CRITICAL_MINUTES = process.env.GH_AW_STEERING_TIME_CRITICAL_MINUTES || "2"; | ||
| process.env.GH_AW_STEERING_RUN_WARNING_REMAINING = process.env.GH_AW_STEERING_RUN_WARNING_REMAINING || "2"; | ||
| process.env.GH_AW_STEERING_RUN_CRITICAL_REMAINING = process.env.GH_AW_STEERING_RUN_CRITICAL_REMAINING || "1"; | ||
|
|
||
| fs.mkdirSync(hooksDir, { recursive: true }); | ||
| const hookConfig = buildSteeringHookConfig(hookScriptPath, process.execPath); | ||
| fs.writeFileSync(hookConfigPath, JSON.stringify(hookConfig, null, 2) + "\n", "utf8"); | ||
| log(`installed steering hook config: ${hookConfigPath}`); | ||
| } catch (error) { | ||
| const err = /** @type {Error} */ error; | ||
| log(`warning: failed to install steering hook config: ${err.message}`); | ||
| } | ||
| } | ||
|
|
||
| function cleanupCopilotSteeringState() { | ||
| const statePath = process.env.GH_AW_COPILOT_STEERING_STATE_PATH || ""; | ||
| if (!statePath) { | ||
| return; | ||
| } | ||
| try { | ||
| if (fs.existsSync(statePath)) { | ||
| fs.unlinkSync(statePath); | ||
| log(`removed steering hook state file: ${statePath}`); | ||
| } | ||
| } catch (error) { | ||
| const err = /** @type {Error} */ error; | ||
| log(`warning: failed to remove steering hook state file ${statePath}: ${err.message}`); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Main entry point: run copilot with retry logic for partially-executed sessions. | ||
| */ | ||
|
|
@@ -310,6 +430,7 @@ async function main() { | |
|
|
||
| await checkCommandAccessible(command); | ||
| const resolvedArgs = resolvePromptFileArgs(args); | ||
| installCopilotSteeringHooks(resolvedArgs); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Smoke test review comment #2 🔍
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oog! Me caveman. Me agree with this observation. Good catch! Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Smoke test review comment #2 🔬
|
||
|
|
||
| // Fetch AWF API proxy reflection data before running the agent to capture initial proxy state. | ||
| // This is best-effort: failures are logged but do not affect the agent run. | ||
|
|
@@ -459,6 +580,7 @@ async function main() { | |
| // This is best-effort: failures are logged but do not affect the agent exit code. | ||
| await fetchAWFReflect({ logger: log }); | ||
|
|
||
| cleanupCopilotSteeringState(); | ||
| log(`done: exitCode=${lastExitCode} totalDuration=${formatDuration(Date.now() - driverStartTime)}`); | ||
| process.exit(lastExitCode); | ||
| } | ||
|
|
@@ -479,13 +601,17 @@ if (typeof module !== "undefined" && module.exports) { | |
| extractModelIds, | ||
| fetchAWFReflect, | ||
| fetchModelsFromUrl, | ||
| buildSteeringHookConfig, | ||
| computeMaxAutopilotRuns, | ||
| resolvePromptFileArgs, | ||
| parseMaxAutopilotContinues, | ||
| }; | ||
| } | ||
|
|
||
| if (require.main === module) { | ||
| main().catch(err => { | ||
| log(`unexpected error: ${err.message}`); | ||
| cleanupCopilotSteeringState(); | ||
| process.exit(1); | ||
| }); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Me like new constants!
DEFAULT_MAX_AUTOPILOT_RUNS = 1make sense as safe default. Clear naming. Me approve!