Skip to content

Commit f2e0645

Browse files
Trecekclaude
andauthored
compose-pr: retry gh pr create with backoff and guard resilience (#4203)
## Summary Add bounded shell-level retry around the Step 5 `gh pr create` command in `src/autoskillit/skills_extended/compose-pr/SKILL.md`. The retry runs only after `gh auth status` has passed, preserves the existing `pr_url = ...` output contract, retries only transient API or network failures, and keeps the PR body closing-issue guard effective for the new command shape. The remediation pass keeps HTTP 429 and secondary rate-limit responses retryable, keeps `compose_pr_body_guard.py` effective for the actual Step 5 `PR_CREATE_BODY` command shape, and reports the final executed attempt's stderr when transient retries are exhausted. Step 5 success is defined by capturing or recovering a PR URL, not by `gh pr create` returning exit code 0 alone. The empty `pr_url =` success path remains reserved for Step 4's `gh auth status` preflight when GitHub CLI is unavailable or unauthenticated. <details> <summary>Individual Group Plans</summary> ### Group 1: Original retry plan Add bounded shell-level retry around the Step 5 `gh pr create` command in [SKILL.md](/home/talon/projects/autoskillit-runs/impl-20260707-054510-064511/src/autoskillit/skills_extended/compose-pr/SKILL.md). The retry should apply only after `gh auth status` has passed, preserve the existing `pr_url = ...` output contract, retry only transient API or network failures, and keep the PR body closing-issue guard effective for the new command shape. ### Group 2: Remediation plan Remediate the remaining `compose-pr` `gh pr create` retry gaps identified by audit-impl. The implementation must keep HTTP 429 and secondary rate-limit responses retryable, keep `compose_pr_body_guard.py` effective for the actual Step 5 `PR_CREATE_BODY` command shape, and report the final executed attempt's stderr when transient retries are exhausted. Step 5 success must be defined by capturing or recovering a PR URL, not by `gh pr create` returning exit code 0 alone. The empty `pr_url =` success path remains reserved for Step 4's `gh auth status` preflight when GitHub CLI is unavailable or unauthenticated. </details> Closes #3987 ## Implementation Plan Plan files: - `/home/talon/projects/autoskillit-runs/impl-20260707-054510-064511/.autoskillit/temp/make-plan/compose_pr_gh_pr_create_retry_plan_2026-07-07_060736.md` - `/home/talon/projects/autoskillit-runs/impl-20260707-054510-064511/.autoskillit/temp/make-plan/compose_pr_retry_remediation_plan_2026-07-07_071159.md` 🤖 Generated with [Claude Code](https://claude.com/claude-code) via AutoSkillit <!-- autoskillit:pipeline-signature steps=prepare_pr,run_arch_lenses,compose_pr,annotate_pr_diff,review_pr --> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent fc7e8b0 commit f2e0645

11 files changed

Lines changed: 1048 additions & 14 deletions

File tree

src/autoskillit/hooks/_command_classification.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,24 @@
4545

4646
_SHELL_OPS: frozenset[str] = frozenset({"&&", "||", ";", "!", "|", "("})
4747

48+
# Shell control words that mark the start of a new command in compound
49+
# shell constructs (loops, conditionals, case statements). When a `gh` token
50+
# is preceded by one of these, treat it as the verb of a fresh command — even
51+
# though shlex does not treat them as operators. Keep this set narrow: only
52+
# words that legitimately precede a command in real shell scripts.
53+
_SHELL_CONTROL_WORDS: frozenset[str] = frozenset(
54+
{
55+
"do",
56+
"done",
57+
"then",
58+
"else",
59+
"elif",
60+
"esac",
61+
"fi",
62+
"in",
63+
}
64+
)
65+
4866
_REDIRECT_TOKEN_RE = re.compile(r"^(\d*)>{1,2}(.+)$")
4967
_REDIRECT_OP_ONLY_RE = re.compile(r"^(\d*)>{1,2}$")
5068
_FD_REDIRECT_RE = re.compile(r"^\d*>{1,2}&")

src/autoskillit/hooks/guards/compose_pr_body_guard.py

Lines changed: 134 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,15 @@
2020
if _HOOKS_DIR not in sys.path:
2121
sys.path.insert(0, _HOOKS_DIR)
2222

23-
from _command_classification import _SHELL_OPS # type: ignore[import-not-found] # noqa: E402
23+
from _command_classification import ( # type: ignore[import-not-found] # noqa: E402
24+
_SHELL_CONTROL_WORDS,
25+
_SHELL_OPS,
26+
)
27+
28+
# Tokens that mark the boundary of a fresh command — either a shell operator
29+
# (already in _SHELL_OPS) or a shell control word like `do`/`then` that
30+
# introduces a new command inside a compound construct (loops, conditionals).
31+
_GH_COMMAND_BOUNDARY: frozenset[str] = _SHELL_OPS | _SHELL_CONTROL_WORDS
2432

2533
COMPOSE_PR_BODY_DENY_TRIGGER: str = "compose-pr body missing Closes reference"
2634

@@ -31,17 +39,132 @@
3139

3240
_METADATA_CLOSING_RE = re.compile(r"^-[ \t]*closing_issue:[ \t]*(\S+)", re.MULTILINE)
3341

42+
_SIMPLE_ASSIGN_RE = re.compile(r"^([A-Za-z_][A-Za-z0-9_]*)=(.*)$")
43+
_VAR_REF_RE = re.compile(r"^\$\{?([A-Za-z_][A-Za-z0-9_]*)\}?$")
44+
_UNSAFE_ASSIGN_VALUE_RE = re.compile(r"[`()|&;]")
45+
_NESTED_VAR_RE = re.compile(r"\$\{?([A-Za-z_][A-Za-z0-9_]*)\}?")
46+
47+
# Keywords that open a nesting level (loop/conditional/case bodies).
48+
_DEPTH_INCREASE: frozenset[str] = frozenset({"while", "for", "until", "if", "case"})
49+
# Keywords that close a nesting level.
50+
_DEPTH_DECREASE: frozenset[str] = frozenset({"done", "fi", "esac"})
51+
52+
53+
def _collect_depth0_assignments(tokens: list[str], before_index: int) -> dict[str, str]:
54+
"""Collect simple variable assignments at nesting depth 0 before *before_index*.
55+
56+
Only assignments with safe values (no command substitution, backticks, or
57+
shell operators in the value) are included. Simple $VAR references in values
58+
are left as-is for downstream resolution.
59+
"""
60+
assignments: dict[str, str] = {}
61+
depth = 0
62+
for i, tok in enumerate(tokens):
63+
if i >= before_index:
64+
break
65+
if tok in _DEPTH_INCREASE:
66+
depth += 1
67+
elif tok in _DEPTH_DECREASE:
68+
if depth > 0:
69+
depth -= 1
70+
elif depth == 0:
71+
m = _SIMPLE_ASSIGN_RE.match(tok)
72+
if m:
73+
name, value = m.group(1), m.group(2)
74+
if not _UNSAFE_ASSIGN_VALUE_RE.search(value):
75+
assignments[name] = value
76+
return assignments
77+
78+
79+
def _resolve_nested_vars(value: str, assignments: dict[str, str]) -> str | None:
80+
"""Expand simple $VAR references in *value* from *assignments*.
81+
82+
Returns the expanded string, or None if any variable cannot be resolved
83+
(fail-open: caller should treat None as "path unknown").
84+
"""
85+
result_parts: list[str] = []
86+
i = 0
87+
while i < len(value):
88+
if value[i] == "$":
89+
m = _NESTED_VAR_RE.match(value, i)
90+
if not m:
91+
return None
92+
var_name = m.group(1)
93+
if var_name not in assignments:
94+
return None
95+
nested_val = assignments[var_name]
96+
if "$" in nested_val:
97+
return None
98+
result_parts.append(nested_val)
99+
i = m.end()
100+
else:
101+
result_parts.append(value[i])
102+
i += 1
103+
return "".join(result_parts)
104+
105+
106+
def _resolve_variable_body_path(raw_token: str, tokens: list[str], gh_index: int) -> str | None:
107+
"""Resolve a $VAR or ${VAR} body-file token from depth-0 assignments before *gh_index*.
108+
109+
Returns the resolved path string, or None if resolution fails (fail-open).
110+
"""
111+
m = _VAR_REF_RE.match(raw_token)
112+
if not m:
113+
return None
114+
var_name = m.group(1)
115+
116+
assignments = _collect_depth0_assignments(tokens, gh_index)
117+
if var_name not in assignments:
118+
return None
119+
120+
raw_value = assignments[var_name]
121+
if "$" not in raw_value:
122+
return raw_value
123+
return _resolve_nested_vars(raw_value, assignments)
124+
125+
126+
def _preprocess_newlines(cmd: str) -> str:
127+
"""Replace bare (unquoted) newlines with ' ; ' to preserve command boundaries.
128+
129+
In shell a bare newline terminates a command just like ';'. shlex.split()
130+
collapses newlines to whitespace, so a later command's --body-file can
131+
bleed into an earlier gh pr create that has none. Replacing unquoted
132+
newlines before tokenisation inserts a proper ';' separator.
133+
"""
134+
result: list[str] = []
135+
in_single = False
136+
in_double = False
137+
i = 0
138+
while i < len(cmd):
139+
c = cmd[i]
140+
if c == "\\" and not in_single and i + 1 < len(cmd):
141+
result.append(c)
142+
result.append(cmd[i + 1])
143+
i += 2
144+
continue
145+
if c == "'" and not in_double:
146+
in_single = not in_single
147+
elif c == '"' and not in_single:
148+
in_double = not in_double
149+
elif c == "\n" and not in_single and not in_double:
150+
result.append(" ; ")
151+
i += 1
152+
continue
153+
result.append(c)
154+
i += 1
155+
return "".join(result)
156+
34157

35158
def _extract_body_file_path(cmd: str) -> str | None:
36159
try:
37-
tokens = shlex.split(cmd)
160+
tokens = shlex.split(_preprocess_newlines(cmd))
38161
except ValueError:
39162
return None
40163

41164
for i, tok in enumerate(tokens):
42165
if tok != "gh":
43166
continue
44-
if i != 0 and tokens[i - 1] not in _SHELL_OPS:
167+
if i != 0 and tokens[i - 1] not in _GH_COMMAND_BOUNDARY:
45168
continue
46169
if i + 2 >= len(tokens) or tokens[i + 1] != "pr" or tokens[i + 2] != "create":
47170
continue
@@ -51,9 +174,15 @@ def _extract_body_file_path(cmd: str) -> str | None:
51174
if t in _SHELL_OPS:
52175
break
53176
if t == "--body-file" and j + 1 < len(tokens) and tokens[j + 1] not in _SHELL_OPS:
54-
return tokens[j + 1]
177+
raw = tokens[j + 1]
178+
if raw.startswith("$"):
179+
return _resolve_variable_body_path(raw, tokens, i)
180+
return raw
55181
if t.startswith("--body-file="):
56-
return t.split("=", 1)[1]
182+
raw = t.split("=", 1)[1]
183+
if raw.startswith("$"):
184+
return _resolve_variable_body_path(raw, tokens, i)
185+
return raw
57186
return None
58187

59188

src/autoskillit/recipe/skill_contracts.yaml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -563,12 +563,13 @@ skills:
563563
- name: pr_url
564564
type: string
565565
expected_output_patterns:
566-
- "pr_url[ \\t]*=[ \\t]*https://github\\.com/.*/pull/\\d+"
566+
- "pr_url[ \\t]*=[ \\t]*(?:https://github\\.com/.*/pull/\\d+)?"
567567
pattern_examples:
568568
- "pr_url = https://github.com/owner/repo/pull/42\n%%ORDER_UP%%"
569+
- "pr_url = \n%%ORDER_UP%%"
569570
write_behavior: conditional
570571
write_expected_when:
571-
- "pr_url[ \\t]*=[ \\t]*https://"
572+
- "pr_url[ \\t]*=[ \\t]*(?:https://|$)"
572573
source_pin_fields:
573574
- field: task_title
574575
required_source: "prep file ## Title section"

src/autoskillit/skills_extended/compose-pr/SKILL.md

Lines changed: 100 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,8 @@ Parse positional arguments:
7777

7878
Derive `feature_branch` and set shell variables:
7979
```bash
80-
FEATURE_BRANCH=$(git -C $WORK_DIR rev-parse --abbrev-ref HEAD)
80+
WORK_DIR=$3
81+
FEATURE_BRANCH=$(git -C "$WORK_DIR" rev-parse --abbrev-ref HEAD)
8182
BASE_BRANCH=$4
8283
```
8384

@@ -230,15 +231,107 @@ If exit code is non-zero:
230231

231232
### Step 5: Create Pull Request
232233

234+
Run `gh pr create` with bounded retry on transient failures. Capture PR URL from stdout.
235+
On success or successful response-loss recovery, emit `pr_url = <URL>`. On exhausted retries or
236+
terminal validation failures, print the final `gh pr create` stderr to stderr and return the
237+
final create status (the empty-`pr_url` path is reserved for Step 4's `gh auth status` preflight).
238+
233239
```bash
234-
gh pr create \
235-
--base $BASE_BRANCH \
236-
--head $FEATURE_BRANCH \
237-
--title "$TASK_TITLE" \
238-
--body-file {{AUTOSKILLIT_TEMP}}/compose-pr/pr_body_$ts.md
240+
PR_CREATE_BODY={{AUTOSKILLIT_TEMP}}/compose-pr/pr_body_$ts.md
241+
PR_CREATE_LOG_DIR={{AUTOSKILLIT_TEMP}}/compose-pr
242+
PR_CREATE_ATTEMPT=1
243+
PR_CREATE_MAX=3
244+
PR_CREATE_STATUS=0
245+
PR_CREATE_LAST_ATTEMPT=1
246+
PR_URL=""
247+
while [ "$PR_CREATE_ATTEMPT" -le "$PR_CREATE_MAX" ]; do
248+
case "$PR_CREATE_ATTEMPT" in
249+
2) sleep 1 ;;
250+
3) sleep 2 ;;
251+
esac
252+
gh pr create \
253+
--base "$BASE_BRANCH" \
254+
--head "$FEATURE_BRANCH" \
255+
--title "$TASK_TITLE" \
256+
--body-file "$PR_CREATE_BODY" \
257+
> "$PR_CREATE_LOG_DIR/pr_stdout_$ts.$PR_CREATE_ATTEMPT.txt" \
258+
2> "$PR_CREATE_LOG_DIR/pr_stderr_$ts.$PR_CREATE_ATTEMPT.txt"
259+
PR_CREATE_STATUS=$?
260+
PR_CREATE_LAST_ATTEMPT=$PR_CREATE_ATTEMPT
261+
if [ "$PR_CREATE_STATUS" -eq 0 ]; then
262+
PR_URL=$(grep -E '^https://github\.com/' "$PR_CREATE_LOG_DIR/pr_stdout_$ts.$PR_CREATE_ATTEMPT.txt" | head -1)
263+
if [ -n "$PR_URL" ]; then
264+
break
265+
fi
266+
fi
267+
PR_CREATE_ERR=$(cat "$PR_CREATE_LOG_DIR/pr_stderr_$ts.$PR_CREATE_ATTEMPT.txt")
268+
case "$PR_CREATE_ERR" in
269+
# Retryable — HTTP 5xx, HTTP 429, secondary rate limit, rate limit, timeout, connection reset/refused.
270+
# Classified before broad HTTP 4xx terminal handling so retryable rate limits are not shadowed.
271+
*HTTP\ 429*|*secondary\ rate\ limit*|*rate\ limit*|*HTTP\ 5*|*timeout*|*connection\ reset*|*connection\ refused*|*temporary\ DNS*|*internal\ server\ error*|*remote\ server\ error*)
272+
: ;;
273+
# Terminal — do not retry (broad HTTP 4xx / validation / required-field missing).
274+
*HTTP\ 4*|*validation*|*required*|*not\ found*)
275+
break ;;
276+
# Ambiguous (empty / unknown stderr) — attempt response-loss recovery, then retry.
277+
*)
278+
: ;;
279+
esac
280+
RECOVERED=$(gh pr view "$FEATURE_BRANCH" --json url -q .url 2>/dev/null || true)
281+
if [ -n "$RECOVERED" ]; then
282+
PR_URL="$RECOVERED"
283+
PR_CREATE_STATUS=0
284+
break
285+
fi
286+
PR_CREATE_ATTEMPT=$((PR_CREATE_ATTEMPT + 1))
287+
done
288+
289+
if [ -n "$PR_URL" ]; then
290+
printf 'pr_url = %s\n' "$PR_URL"
291+
exit 0
292+
fi
293+
294+
if [ "$PR_CREATE_STATUS" -eq 0 ]; then
295+
PR_CREATE_STATUS=1
296+
fi
297+
echo "gh pr create failed after ${PR_CREATE_LAST_ATTEMPT} attempt(s):" >&2
298+
LAST_STDERR="$PR_CREATE_LOG_DIR/pr_stderr_$ts.$PR_CREATE_LAST_ATTEMPT.txt"
299+
if [ -s "$LAST_STDERR" ]; then
300+
cat "$LAST_STDERR" >&2
301+
else
302+
echo "gh pr create exited 0 but returned no PR URL after ${PR_CREATE_LAST_ATTEMPT} attempt(s)." >&2
303+
fi
304+
exit "$PR_CREATE_STATUS"
239305
```
240306

241-
Capture PR URL from stdout.
307+
Bounded retry policy:
308+
- Maximum 3 total `gh pr create` attempts
309+
- Backoff sleeps of 1 second before attempt 2 and 2 seconds before attempt 3
310+
- Each attempt's stdout/stderr captured under `{{AUTOSKILLIT_TEMP}}/compose-pr/`
311+
so final diagnostics can print useful detail without contaminating the PR URL capture
312+
- Retryable rate-limit patterns (`HTTP 429`, `secondary rate limit`, broad `rate limit`) are
313+
classified before the broad terminal `HTTP 4xx` pattern so they are never shadowed
314+
- Retry only transient or ambiguous response-loss failures (HTTP 429, secondary rate limit,
315+
broad rate limit, HTTP 5xx, timeout, connection reset/refused, temporary DNS resolution,
316+
remote/internal server errors)
317+
- Do not retry terminal validation or usage failures (broad HTTP 4xx, validation errors,
318+
missing required fields, not-found errors)
319+
320+
Response-loss recovery: when a create attempt fails with a transient/ambiguous status (or empty
321+
stderr that suggests the create succeeded but the response was lost), run a best-effort
322+
`gh pr view "$FEATURE_BRANCH" --json url -q .url` before sleeping and retrying. If it returns
323+
a URL, treat the create as recovered and emit that URL. The recovery path runs only for
324+
transient/ambiguous failures — never for terminal validation failures.
325+
326+
Last-attempt tracking: `PR_CREATE_LAST_ATTEMPT` is set to `$PR_CREATE_ATTEMPT` immediately
327+
after each `gh pr create` execution. Final failure reporting reads the stderr file for
328+
`PR_CREATE_LAST_ATTEMPT`, not the post-increment loop counter.
329+
330+
Final status propagation: if the loop exits without a captured or recovered PR URL, the exit
331+
status is forced nonzero even if the last raw `gh pr create` exit code was 0 (zero-exit/no-URL
332+
is not a successful outcome). When the last attempt's stderr file is empty or absent (zero-exit
333+
path), a concise no-URL diagnostic is printed instead. The empty-`pr_url` success path is
334+
reserved exclusively for Step 4's `gh auth status` preflight failure.
242335

243336
## Output
244337

tests/_test_filter.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -802,6 +802,7 @@ class ImportContext(enum.StrEnum):
802802
"skills/test_audit_impl_diff_discipline.py",
803803
"skills/test_skill_variable_threading.py",
804804
"skills/test_phoropter_structural.py",
805+
"skills/test_compose_pr_retry.py",
805806
"smoke_utils",
806807
}
807808
),

0 commit comments

Comments
 (0)