Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion .github/workflows/dev-lead-reusable.yml
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,14 @@ jobs:
(inputs.event_name == '' && github.event_name != 'check_run') ||
(inputs.event_name != '' && inputs.event_name != 'check_run')
runs-on: ubuntu-latest
timeout-minutes: 60
# Job backstop for a hung/stuck run — NOT a cost control (#1017, epic #901).
# 120 min gives legitimately large issues room to finish across all engine
# tiers before the backstop fires (GitHub's hard cap is 360/job). The
# effective per-tier defaults live in engine.sh; scripts/dev-lead-timeout-
# guardrail.sh asserts Σ(per-tier budgets)+overhead ≤ this value so tiers can
# never silently blow past the backstop. DEEP_TIMEOUT_SEC / ACTION_TIMEOUT_SEC
# below still pass caller `vars` overrides through; the default is in engine.sh.
timeout-minutes: 120
permissions:
contents: write
pull-requests: write
Expand Down
98 changes: 98 additions & 0 deletions scripts/dev-lead-timeout-guardrail.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
#!/usr/bin/env bash
# dev-lead-timeout-guardrail.sh — #1017 (epic #901, story A/3)
#
# Static guardrail: assert that the sum of the per-tier engine timeouts plus a
# fixed overhead margin can never exceed the dev-lead job's `timeout-minutes`
# backstop. The job cap is a hang/stuck-run backstop (GitHub hard cap 360/job);
# per-tier timeouts stop a single hung model invocation from burning it. If the
# tiers could sum past the job cap, a legitimate multi-tier run could be killed
# mid-write by the job backstop instead of by its own tier timeout. This check
# fails CI loudly if that invariant is ever violated by a config edit.
#
# Invariant: Σ(per-tier default budgets) + OVERHEAD_MARGIN_SEC ≤ job_cap_sec
#
# Overhead margin accounts for the fixed, non-tier work in the job (repo
# checkout, CLI install, git/PR operations) — empirically ~10–15 min.
#
# Sources of truth (parsed, never hardcoded here):
# - per-tier defaults: scripts/engine.sh (`VAR="${VAR:-N}"`)
# - job cap: .github/workflows/dev-lead-reusable.yml (dispatch job
# `timeout-minutes:`)
#
# Overridable via env for testing: ENGINE_SH, WORKFLOW_YML, OVERHEAD_MARGIN_SEC.
set -euo pipefail

_here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
_repo_root="$(cd "$_here/.." && pwd)"

ENGINE_SH="${ENGINE_SH:-$_repo_root/scripts/engine.sh}"
WORKFLOW_YML="${WORKFLOW_YML:-$_repo_root/.github/workflows/dev-lead-reusable.yml}"
# Fixed overhead margin (seconds): checkout + CLI install + git/PR ops.
OVERHEAD_MARGIN_SEC="${OVERHEAD_MARGIN_SEC:-900}"

_die() { echo "guardrail: $*" >&2; exit 2; }

[ -r "$ENGINE_SH" ] || _die "cannot read engine.sh at $ENGINE_SH"
[ -r "$WORKFLOW_YML" ] || _die "cannot read workflow at $WORKFLOW_YML"

# Extract a `VAR="${VAR:-N}"` default from engine.sh. Skips commented-out lines
# and tolerates leading whitespace so a stray comment can't be mis-parsed.
_tier_default() {
local var="$1" val
val="$(sed -n "/^[[:space:]]*#/d; s/^[[:space:]]*$var=\"\${$var:-\([0-9][0-9]*\)}\".*/\1/p" "$ENGINE_SH")"
[ -n "$val" ] || _die "could not parse default for $var in $ENGINE_SH"
printf '%s' "$val"
}

# Extract the dispatch job's `timeout-minutes:` (minutes). The dispatch job is
# the one the engine tiers run under; the ci-relay job's own timeout is
# irrelevant to tier budgeting.
_job_timeout_min() {
local val
val="$(awk '
/^[[:space:]]*#/ {next}
/^ dispatch:[[:space:]]*$/ {inblk=1; next}
inblk==1 && /^ [A-Za-z]/ {inblk=0}
inblk==1 && /^[[:space:]]*timeout-minutes:/ {print $2; exit}
' "$WORKFLOW_YML")"
Comment thread
don-petry marked this conversation as resolved.
[ -n "$val" ] || _die "could not parse dispatch timeout-minutes in $WORKFLOW_YML"
printf '%s' "$val"
}

TRIAGE="$(_tier_default TRIAGE_TIMEOUT_SEC)"
DEEP="$(_tier_default DEEP_TIMEOUT_SEC)"
AUDIT="$(_tier_default AUDIT_TIMEOUT_SEC)"
ACTION="$(_tier_default ACTION_TIMEOUT_SEC)"
DUCK="$(_tier_default DUCK_TIMEOUT_SEC)"

job_min="$(_job_timeout_min)"

# Fail with a clear guardrail message (not a generic bash arithmetic error) if
# any budget/timeout failed to parse as a positive integer — e.g. a tier default
# is missing or timeout-minutes became a non-numeric expression.
for _v in "$TRIAGE" "$DEEP" "$AUDIT" "$ACTION" "$DUCK" "$job_min"; do
[[ "$_v" =~ ^[0-9]+$ ]] || _die "failed to parse a numeric budget/timeout (got: '${_v}') — check $ENGINE_SH and $WORKFLOW_YML"
done

job_sec=$(( job_min * 60 ))

sigma=$(( TRIAGE + DEEP + AUDIT + ACTION + DUCK ))
required=$(( sigma + OVERHEAD_MARGIN_SEC ))

echo "dev-lead timeout guardrail (#1017)"
echo " per-tier budgets (sec): TRIAGE=$TRIAGE DEEP=$DEEP AUDIT=$AUDIT ACTION=$ACTION DUCK=$DUCK"
echo " Σ tiers = ${sigma}s"
echo " overhead margin = ${OVERHEAD_MARGIN_SEC}s"
echo " Σ + overhead = ${required}s"
echo " job timeout = ${job_min}m = ${job_sec}s"

if [ "$required" -le "$job_sec" ]; then
echo " RESULT: PASS (Σ+overhead ${required}s ≤ job ${job_sec}s)"
exit 0
fi

echo " RESULT: FAIL — per-tier budgets exceed the job backstop" >&2
echo " Σ+overhead ${required}s > job ${job_sec}s (over by $(( required - job_sec ))s)" >&2
echo " Fix: lower a per-tier *_TIMEOUT_SEC default in engine.sh or raise the" >&2
echo " dispatch job timeout-minutes in dev-lead-reusable.yml (cap 360)." >&2
exit 1
27 changes: 21 additions & 6 deletions scripts/engine.sh
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,28 @@ DEFAULT_COPILOT_API_MODEL="openai/o4-mini"
COPILOT_API_MODEL="${COPILOT_API_MODEL:-$DEFAULT_COPILOT_API_MODEL}"
export COPILOT_API_MODEL

# Per-tier timeouts (seconds). The job-level 60min cap is a backstop — without
# per-tier timeouts a single hung model invocation burns the whole hour and
# blocks every subsequent PR in the session.
# Per-tier timeouts (seconds). The job-level cap (120min in dev-lead-reusable.yml)
# is a hang/stuck-run backstop — without per-tier timeouts a single hung model
# invocation burns the whole job and blocks every subsequent PR in the session.
# These are sized to MAXIMIZE legitimate runtime per stage (#1017, epic #901),
# NOT to bound token/runner cost. DEEP (deep agentic analysis) and ACTION (the
# writer tier) get the largest viable share; TRIAGE/DUCK stay small (lightweight
# read/classify + cross-engine review); AUDIT stays modest.
#
# Guardrail invariant (scripts/dev-lead-timeout-guardrail.sh + its bats test):
# Σ(these defaults) + overhead_margin(900s) ≤ job timeout-minutes × 60
# 300 + 2400 + 1200 + 2100 + 300 = 6300s; 6300 + 900 = 7200s = 120min ✓
# so the tiers can never silently exceed the job backstop. Raising a tier here
# requires lowering another or raising the job cap, or the guardrail fails CI.
#
# Fleet note: petry-projects/.github leaves `vars.DEEP_TIMEOUT_SEC` (and
# ACTION_TIMEOUT_SEC) UNSET, so the workflow passes an empty value through and
# these engine defaults take effect. The raised DEEP default (was 600, now 2400)
# therefore applies fleet-wide unless a repo overrides it via its own `vars`.
TRIAGE_TIMEOUT_SEC="${TRIAGE_TIMEOUT_SEC:-300}"
DEEP_TIMEOUT_SEC="${DEEP_TIMEOUT_SEC:-600}"
AUDIT_TIMEOUT_SEC="${AUDIT_TIMEOUT_SEC:-600}"
ACTION_TIMEOUT_SEC="${ACTION_TIMEOUT_SEC:-600}"
DEEP_TIMEOUT_SEC="${DEEP_TIMEOUT_SEC:-2400}"
AUDIT_TIMEOUT_SEC="${AUDIT_TIMEOUT_SEC:-1200}"
ACTION_TIMEOUT_SEC="${ACTION_TIMEOUT_SEC:-2100}"
DUCK_TIMEOUT_SEC="${DUCK_TIMEOUT_SEC:-300}"

# Retry config for transient errors. We treat exit codes that look like
Expand Down
90 changes: 90 additions & 0 deletions tests/dev-lead/unit/test_timeout_guardrail.bats
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
#!/usr/bin/env bats
# Guardrail for #1017 (epic #901, story A/3): the per-tier engine timeouts plus a
# fixed overhead margin must never exceed the dev-lead job's `timeout-minutes`
# backstop, so a hung tier can never silently blow past the job cap. This pins
# the raised budgets (DEEP/ACTION/AUDIT) and the raised job cap (120) against
# accidental drift, and proves the guardrail actually *fails* when a tier is
# inflated past the cap.

REPO_ROOT="$(cd "$BATS_TEST_DIRNAME"/../../.. && pwd)"
GUARDRAIL="$REPO_ROOT/scripts/dev-lead-timeout-guardrail.sh"
ENGINE="$REPO_ROOT/scripts/engine.sh"
WORKFLOW="$REPO_ROOT/.github/workflows/dev-lead-reusable.yml"

# Extract a `VAR="${VAR:-N}"` default from engine.sh. Skips comment lines and
# tolerates leading whitespace (mirrors the guardrail script's parser).
_tier_default() {
sed -n "/^[[:space:]]*#/d; s/^[[:space:]]*$1=\"\${$1:-\([0-9][0-9]*\)}\".*/\1/p" "$ENGINE"
}
Comment thread
don-petry marked this conversation as resolved.

@test "guardrail: script exists and is executable" {
[ -f "$GUARDRAIL" ]
[ -x "$GUARDRAIL" ]
}

@test "guardrail: passes against the committed engine + workflow config" {
run bash "$GUARDRAIL"
[ "$status" -eq 0 ] || { echo "$output"; return 1; }
}

# Parse the dispatch job's timeout-minutes directly (no bash -c wrapper).
_dispatch_timeout_min() {
awk '
/^[[:space:]]*#/ {next}
/^ dispatch:[[:space:]]*$/ {inblk=1; next}
inblk==1 && /^ [A-Za-z]/ {inblk=0}
inblk==1 && /^[[:space:]]*timeout-minutes:/ {print $2; exit}
' "$WORKFLOW"
}

@test "guardrail: dispatch job timeout-minutes raised to 120" {
run _dispatch_timeout_min
[ "$status" -eq 0 ]
[ "$output" = "120" ]
}

@test "guardrail: DEEP and ACTION defaults raised above the old 600 baseline" {
local deep action
deep="$(_tier_default DEEP_TIMEOUT_SEC)"
action="$(_tier_default ACTION_TIMEOUT_SEC)"
[ -n "$deep" ] && [ -n "$action" ]
[ "$deep" -gt 600 ]
[ "$action" -gt 600 ]
}

@test "guardrail: TRIAGE and DUCK stay lightweight (unchanged small budgets)" {
[ "$(_tier_default TRIAGE_TIMEOUT_SEC)" -le 600 ]
[ "$(_tier_default DUCK_TIMEOUT_SEC)" -le 600 ]
}

@test "guardrail: Sigma(tier budgets) + overhead margin <= job cap" {
# Independently recompute the invariant from the parsed config (not by
# grepping the script's own PASS line), then confirm the script agrees.
local triage deep audit action duck job_min sigma required cap
triage="$(_tier_default TRIAGE_TIMEOUT_SEC)"
deep="$(_tier_default DEEP_TIMEOUT_SEC)"
audit="$(_tier_default AUDIT_TIMEOUT_SEC)"
action="$(_tier_default ACTION_TIMEOUT_SEC)"
duck="$(_tier_default DUCK_TIMEOUT_SEC)"
job_min="$(_dispatch_timeout_min)"
sigma=$(( triage + deep + audit + action + duck ))
# 900 = the guardrail script's default OVERHEAD_MARGIN_SEC.
required=$(( sigma + 900 ))
cap=$(( job_min * 60 ))
[ "$required" -le "$cap" ]
# …and the script itself reaches the same verdict.
run bash "$GUARDRAIL"
[ "$status" -eq 0 ]
}

@test "guardrail: FAILS when a per-tier budget is inflated past the job cap" {
# Point the guardrail at a doctored engine.sh whose DEEP budget alone exceeds
# the job cap; the check must reject it (non-zero exit) rather than pass.
local tmp
tmp="$(mktemp)"
sed 's/^DEEP_TIMEOUT_SEC="\${DEEP_TIMEOUT_SEC:-[0-9]*}"/DEEP_TIMEOUT_SEC="${DEEP_TIMEOUT_SEC:-99999}"/' "$ENGINE" > "$tmp"
run env ENGINE_SH="$tmp" bash "$GUARDRAIL"
rm -f "$tmp"
[ "$status" -ne 0 ]
echo "$output" | grep -qiE "FAIL|exceed"
}
Loading