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
4 changes: 2 additions & 2 deletions .github/workflows/design-decision-gate.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

36 changes: 22 additions & 14 deletions .github/workflows/design-decision-gate.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,20 +81,35 @@ steps:
run: |
set -euo pipefail

mkdir -p /tmp/gh-aw/agent
SKIP_REASON=""
if [ "$EXPR_GITHUB_EVENT_NAME" = "workflow_dispatch" ] && [ -z "${PR_NUMBER:-}" ]; then
echo "::notice::workflow_dispatch did not include inputs.pr_number; skipping ADR gate."
mkdir -p /tmp/gh-aw/agent
SKIP_REASON="workflow_dispatch did not include inputs.pr_number"
printf '{"number":null,"title":"No pull request provided","body":"","labels":[],"baseRefName":"","headRefName":"","author":null,"url":""}\n' \
> /tmp/gh-aw/agent/pr.json
else
gh pr view "$PR_NUMBER" \
--repo "$EXPR_GITHUB_REPOSITORY" \
--json number,title,body,labels,baseRefName,headRefName,author,url,state \
> /tmp/gh-aw/agent/pr.json
PR_STATE=$(jq -r '.state' /tmp/gh-aw/agent/pr.json)
if [ "$PR_STATE" = "CLOSED" ] || [ "$PR_STATE" = "MERGED" ]; then
SKIP_REASON="Pull request #$PR_NUMBER is $PR_STATE; ADR enforcement applies only to open pull requests"
fi
fi

if [ -n "$SKIP_REASON" ]; then
echo "::notice::$SKIP_REASON; skipping ADR gate."
printf '[]\n' > /tmp/gh-aw/agent/pr-files.json
printf '# ADR gate skipped: workflow_dispatch did not include inputs.pr_number.\n' \
printf '# ADR gate skipped: %s.\n' "$SKIP_REASON" \
> /tmp/gh-aw/agent/pr.diff
printf 'No .design-gate.yml read because workflow_dispatch did not include inputs.pr_number.\n' \
printf 'No .design-gate.yml read because ADR gate was skipped: %s.\n' "$SKIP_REASON" \
> /tmp/gh-aw/agent/design-gate-config.yml
jq -n \
--arg skip_reason "workflow_dispatch did not include inputs.pr_number" \
--arg skip_reason "$SKIP_REASON" \
--arg pr_number "${PR_NUMBER:-}" \
'{
pr_number: null,
pr_number: (if $pr_number == "" then null else ($pr_number | tonumber) end),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This skip path coerces the raw workflow_dispatch input with tonumber, which will abort prefetch if the operator passes a PR URL/ref that gh pr view resolves successfully.

💡 Why this is a real regression

workflow_dispatch inputs are untyped strings. gh pr view "$PR_NUMBER" accepts more than bare numeric IDs, so a closed PR supplied as a URL or another resolvable selector reaches this branch and then dies while writing adr-prefetch-summary.json. That turns the intended noop skip into a hard failure.

Use the validated .number already returned in pr.json, or guard the conversion before calling tonumber.

threshold: 100,
has_custom_config: false,
has_implementation_label: false,
Expand All @@ -107,13 +122,6 @@ steps:
exit 0
fi

mkdir -p /tmp/gh-aw/agent

gh pr view "$PR_NUMBER" \
--repo "$EXPR_GITHUB_REPOSITORY" \
--json number,title,body,labels,baseRefName,headRefName,author,url \
> /tmp/gh-aw/agent/pr.json

gh api --paginate "repos/$EXPR_GITHUB_REPOSITORY/pulls/$PR_NUMBER/files?per_page=100" \
--jq '.[]' | jq -s '.' > /tmp/gh-aw/agent/pr-files.json

Expand Down Expand Up @@ -197,7 +205,7 @@ Stop at the first step where you have sufficient information to emit a safe outp

Stop and emit a safe output **immediately** when any of the following is true:

- **Skipped prefetch**: `adr-prefetch-summary.json` contains `skip_reason` → call `noop` with that reason and **stop**.
- **Skipped prefetch**: `adr-prefetch-summary.json` contains `skip_reason` (including a PR that has already closed or merged) → call `noop` with that reason and **stop**. Do not generate an ADR or attempt to push to a closed PR's branch.
- **Noop exit**: `has_implementation_label` is `false` AND `requires_adr_by_default_volume` is `false` → call `noop` and **stop**.
- **ADR found, no divergence**: ADR contains all four required sections and the diff does not contradict the decision → call `add-comment` (approved) and **stop**.
- **ADR found, divergence**: Divergences identified → call `add-comment` (divergence list) and **stop**.
Expand Down
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,7 @@ test-scripts: build
bash scripts/check-safe-outputs-conformance_test.sh
bash scripts/resolve-base-commit_test.sh
bash scripts/pr-data-prefetch_test.sh
bash scripts/design-decision-gate-prefetch_test.sh
bash scripts/check-workflow-drift_test.sh ./$(BINARY_NAME)
bash scripts/check-cgo-cjs-workflow-purity_test.sh
bash scripts/update-threat-detect-pins_test.sh
Expand Down
91 changes: 91 additions & 0 deletions scripts/design-decision-gate-prefetch_test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
#!/usr/bin/env bash
set -euo pipefail

ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
TEST_DIR="$(mktemp -d)"
trap 'rm -rf "$TEST_DIR"' EXIT
mkdir -p "$TEST_DIR/bin" "$TEST_DIR/agent" "$TEST_DIR/workspace"

awk '
/^ run: \|$/ { body = 1; next }
body && /^ / { sub(/^ /, ""); print; next }
body && /^$/ { print; next }
body { exit }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This harness reads the markdown source instead of the compiled .lock.yml, so the test can pass while the workflow GitHub actually runs still contains stale prefetch logic.

💡 Why this blocks the fix

The regression you are fixing is in the production workflow, but this test extracts the shell body from .github/workflows/design-decision-gate.md. If the lockfile is stale or recompilation is skipped, CI still goes green here while the shipped workflow keeps the old behavior.

A safer pattern is to exercise design-decision-gate.lock.yml directly, or assert that the compiled lock step matches the markdown source before running the harness. That way this test actually proves the runnable workflow was updated.

' "$ROOT/.github/workflows/design-decision-gate.md" |
sed "s|/tmp/gh-aw/agent|$TEST_DIR/agent|g" > "$TEST_DIR/prefetch.sh"
test -s "$TEST_DIR/prefetch.sh"

cat > "$TEST_DIR/bin/gh" <<'MOCK'
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' "$*" >> "$TEST_DIR/calls"
case "$*" in
"pr view "*)
[[ "$*" == *",state" ]]
if [ "$FAIL_PR" = 1 ]; then
echo 'gh: API request failed (HTTP 403)' >&2
exit 1
fi
jq -n --arg state "$PR_STATE" \
'{number:42,title:"Implementation",body:"",labels:[],state:$state,
baseRefName:"main",headRefName:"feature",author:null,url:""}'
;;
"api --paginate repos/owner/repo/pulls/42/files?per_page=100 --jq .[]")
printf '{"filename":"pkg/implementation.go","additions":553}\n'
;;
"pr diff 42 --repo owner/repo")
printf 'diff --git a/pkg/implementation.go b/pkg/implementation.go\n'
;;
*)
echo "Unexpected API request: $*" >&2
exit 1
;;
esac
MOCK
chmod +x "$TEST_DIR/bin/gh"
export TEST_DIR PATH="$TEST_DIR/bin:$PATH"
export EXPR_GITHUB_EVENT_NAME=pull_request EXPR_GITHUB_REPOSITORY=owner/repo
export EXPR_GITHUB_WORKSPACE="$TEST_DIR/workspace" PR_NUMBER=42 PR_STATE=OPEN FAIL_PR=0
unset GH_TOKEN GITHUB_TOKEN

run_prefetch() {
: > "$TEST_DIR/calls"
rm -f "$TEST_DIR/agent/adr-prefetch-summary.json"
bash "$TEST_DIR/prefetch.sh" > "$TEST_DIR/output" 2>&1
}

# An open PR still requires an ADR for the same 553-line implementation.
run_prefetch
test "$(wc -l < "$TEST_DIR/calls" | tr -d ' ')" = 3
jq -e '.pr_number == 42 and .requires_adr_by_default_volume and
.default_business_additions == 553 and .diff_available and (has("skip_reason") | not)' \
"$TEST_DIR/agent/adr-prefetch-summary.json" >/dev/null

# A stale open webhook must not cause ADR generation on a closed/deleted branch.
for state in CLOSED MERGED; do
PR_STATE="$state" run_prefetch
test "$(wc -l < "$TEST_DIR/calls" | tr -d ' ')" = 1
jq -e --arg state "$state" '.pr_number == 42 and
(.skip_reason | contains($state)) and .file_count == 0 and (.diff_available | not)' \
"$TEST_DIR/agent/adr-prefetch-summary.json" >/dev/null
jq -e 'length == 0' "$TEST_DIR/agent/pr-files.json" >/dev/null
grep -q '::notice::Pull request #42' "$TEST_DIR/output"
done

# Preserve manual dispatch behavior, both with and without a PR number.
EXPR_GITHUB_EVENT_NAME=workflow_dispatch PR_NUMBER="" run_prefetch
test ! -s "$TEST_DIR/calls"
jq -e '.pr_number == null and .skip_reason == "workflow_dispatch did not include inputs.pr_number"' \
"$TEST_DIR/agent/adr-prefetch-summary.json" >/dev/null
EXPR_GITHUB_EVENT_NAME=workflow_dispatch run_prefetch
jq -e '.requires_adr_by_default_volume and (has("skip_reason") | not)' \
"$TEST_DIR/agent/adr-prefetch-summary.json" >/dev/null

# API failures cannot turn an unverified PR into a successful skip.
if FAIL_PR=1 run_prefetch; then
echo 'PR metadata failure unexpectedly succeeded' >&2
exit 1
fi
test ! -e "$TEST_DIR/agent/adr-prefetch-summary.json"
grep -q 'HTTP 403' "$TEST_DIR/output"
echo 'Design Decision Gate prefetch regressions passed'
Loading