Skip to content

Commit 8f8119b

Browse files
authored
Fix Contribution Check proxy denials and retry handling (#51631)
1 parent 6ff33a5 commit 8f8119b

5 files changed

Lines changed: 40 additions & 5 deletions

File tree

.github/workflows/contribution-check.lock.yml

Lines changed: 4 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.github/workflows/contribution-check.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ sandbox:
2828
sudo: false
2929
tools:
3030
cli-proxy: true
31-
bash: ["cat", "ls", "find", "grep", "head", "tail", "wc"]
31+
bash: ["cat", "ls", "find", "grep", "head", "tail", "wc", "git", "jq *"]
3232
github:
3333
mode: gh-proxy
3434
toolsets: [pull_requests, repos, issues]

actions/setup/js/copilot_harness.cjs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1287,10 +1287,9 @@ async function main() {
12871287
log(`attempt ${attempt + 1}: AI credits marker found in CLI output without trusted firewall audit confirmation — preserving normal failure handling`);
12881288
}
12891289
const shouldTreatAICreditsExceededAsSuccess = trustedAICreditsExceeded && !isAuthenticationFailed;
1290-
if (shouldTreatAICreditsExceededAsSuccess || nonRetryableGuard.awfAPIProxyBlockingRequests || isInvocationCapExceeded) {
1290+
if (shouldTreatAICreditsExceededAsSuccess || isInvocationCapExceeded) {
12911291
const reasons = [];
12921292
if (shouldTreatAICreditsExceededAsSuccess) reasons.push("AI credits budget exceeded");
1293-
if (nonRetryableGuard.awfAPIProxyBlockingRequests) reasons.push("AWF API proxy is blocking requests");
12941293
if (isInvocationCapExceeded) {
12951294
reasons.push("LLM invocation cap saturated — the pooled per-run budget is fully exhausted; retries cannot make progress");
12961295
}

actions/setup/js/copilot_harness.test.cjs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,12 @@ describe("copilot_harness.cjs", () => {
335335
expect(shouldRetry(result, 0)).toBe(false);
336336
});
337337

338+
it("retries AWF API proxy blocks instead of treating them as a guard condition", () => {
339+
const result = { exitCode: 1, hasOutput: true, output: "awf api proxy is blocking requests for this run" };
340+
expect(detectNonRetryableHarnessGuard(result.output).awfAPIProxyBlockingRequests).toBe(true);
341+
expect(shouldRetry(result, 0)).toBe(true);
342+
});
343+
338344
it("does not retry the observed CAPIError 429 quota exceeded error even when session produced output", () => {
339345
const result = {
340346
exitCode: 1,

pkg/cli/contribution_check_workflow_contract_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,3 +34,31 @@ func TestContributionCheckWorkflowSafeOutputContract(t *testing.T) {
3434
assert.Contains(t, text, "\"issue_number\":35304", "Workflow should include a concrete add_comment issue_number example")
3535
assert.Contains(t, text, "model: claude-haiku-4.5", "Workflow should require small model for contribution-checker subagent calls")
3636
}
37+
38+
func TestContributionCheckWorkflowAllowsRequiredShellCommands(t *testing.T) {
39+
repoRoot, err := gitutil.FindGitRoot()
40+
if err != nil {
41+
t.Skipf("Skipping test: not in a git repository: %v", err)
42+
}
43+
44+
workflowPath := filepath.Join(repoRoot, ".github", "workflows", "contribution-check.md")
45+
content, err := os.ReadFile(workflowPath)
46+
require.NoError(t, err, "Should read contribution-check workflow")
47+
48+
text := string(content)
49+
assert.Contains(t, text, `"git"`, "Workflow must allow git fetch/diff commands used by contribution-checker subagents")
50+
assert.Contains(t, text, `"jq *"`, "Workflow must allow jq payload construction for safeoutputs create_issue")
51+
52+
lockPath := filepath.Join(repoRoot, ".github", "workflows", "contribution-check.lock.yml")
53+
lockContent, err := os.ReadFile(lockPath)
54+
require.NoError(t, err, "Should read compiled contribution-check workflow")
55+
56+
lockText := string(lockContent)
57+
for _, token := range []string{
58+
"--allow-tool '\\''shell(git:*)'\\''",
59+
"--allow-tool '\\''shell(jq)'\\''",
60+
"--allow-tool '\\''shell(safeoutputs:*)'\\''",
61+
} {
62+
assert.Containsf(t, lockText, token, "Compiled workflow must contain %s", token)
63+
}
64+
}

0 commit comments

Comments
 (0)