From edb43ece285832a1f149e2c0c4a13399ddc69299 Mon Sep 17 00:00:00 2001 From: coder-abt Date: Tue, 11 Aug 2026 03:34:37 +0500 Subject: [PATCH 1/3] ci(ai-sdlc): run the spec guards on PRs, not only inside the agent `verify-spec-ownership.mjs` and `check-spec-scope.mjs` ran only inside spec-agent.yml. `ci.yml` runs `node --test scripts/ai-sdlc/*.test.mjs` - it tests the guards, it never invokes them. So they guarded the machine path and not the change: - the spec ratification PR (Gate 1) never re-ran ownership on the version a human edits and merges; - PR #283, the delivery for issue #269, was written by hand and passed through none of them. So did 13bf6b3 and c0af5ec. The guardrails were attached to the machine path while the delivery went down the human path. This is the second time that inversion has shown up - the first was impl-agent.yml hardcoding `Refs #%s`, which makes agent PRs structurally incapable of skipping Gate 4 while hand-written ones can. Runs on `docs/specs/**` PRs only, so it costs nothing on the rest. Ownership is hard, scope stays advisory - and moving scope here is most of its value, since its warning previously landed in the spec-agent run's step summary rather than on the PR the reviewer is actually reading. Verified against real specs rather than assumed: spec 0237 gives "ownership lint passed - 4 declared path(s) checked" and "4 file(s) declared, within cap (6)". Spec 0226 has no `Files touched:` line and both guards skip with exit 0 - that fail-open path is pre-existing and is noted in the follow-up issue rather than silently relied on here. Deliberately spec-side only. The impl equivalent needs a PR-diff-level comparison that check-impl-scope.mjs explicitly declines to attempt (its own header: "does not re-check later commits a human pushes to the same PR before merge - that's a different, PR-diff-level check this script does not attempt"), plus spec resolution from the linked issue. That is new logic rather than a relocation, so it is filed separately. actionlint clean. Assisted-by: claude-opus-5 (agent) Co-authored-by: Claude Opus 5 (1M context) --- .github/workflows/ai-sdlc-guards.yml | 105 +++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 .github/workflows/ai-sdlc-guards.yml diff --git a/.github/workflows/ai-sdlc-guards.yml b/.github/workflows/ai-sdlc-guards.yml new file mode 100644 index 00000000..50cfbd49 --- /dev/null +++ b/.github/workflows/ai-sdlc-guards.yml @@ -0,0 +1,105 @@ +name: AI-SDLC Guards + +# The scope and ownership guards used to run ONLY inside spec-agent.yml and +# impl-agent.yml, which meant they guarded the machine path and not the change. +# ci.yml runs `node --test scripts/ai-sdlc/*.test.mjs` - it tests the guards, it +# never invokes them. Consequences, all real: +# +# - the spec ratification PR (Gate 1) did not re-run ownership on the version a +# human edits and merges; +# - PR #283, the delivery for issue #269, was written by hand and therefore +# passed through none of them, as did 13b3f6b and c0af5ec. +# +# The guardrails were attached to the machine path; the delivery went down the +# human path. This puts the spec-side guards on the PR, where the change is. +# +# Scope, stated honestly: spec-side only. The impl-side equivalent needs a +# PR-diff-level comparison that check-impl-scope.mjs explicitly does not attempt +# (see its own header) plus spec resolution from the linked issue. That is new +# logic rather than a relocation, and it is filed separately. + +on: + pull_request: + types: [opened, synchronize, reopened, ready_for_review] + paths: + - 'docs/specs/**' + +permissions: + contents: read + +concurrency: + group: ai-sdlc-guards-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + spec-guards: + name: Spec guards + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Checkout PR merge result + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Collect spec files changed by this PR + id: specs + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + set -euo pipefail + # Added or modified only. A deleted spec has nothing to verify, and + # feeding a deleted path to the guards would fail on the read rather + # than on the rule, which is a misleading way to go red. + files=$(git diff --name-only --diff-filter=AM "$BASE_SHA" "$HEAD_SHA" -- 'docs/specs/*.md' || true) + if [ -z "$files" ]; then + echo "count=0" >> "$GITHUB_OUTPUT" + echo "No added or modified spec files; nothing to check." + exit 0 + fi + echo "count=$(printf '%s\n' "$files" | wc -l)" >> "$GITHUB_OUTPUT" + # Multi-line value needs a heredoc delimiter, or the runner rejects + # the whole step. + { + echo 'files<> "$GITHUB_OUTPUT" + printf '%s\n' "$files" | sed 's/^/ /' + + - name: Verify spec path ownership (deterministic, hard) + if: steps.specs.outputs.count != '0' + env: + SPECS: ${{ steps.specs.outputs.files }} + run: | + set -euo pipefail + failed=0 + while IFS= read -r spec; do + [ -n "$spec" ] || continue + echo "::group::verify-spec-ownership $spec" + # Run every spec before failing, so one PR touching two specs + # reports both rather than only the first. + SPEC_FILE="$spec" node scripts/ai-sdlc/verify-spec-ownership.mjs || failed=1 + echo "::endgroup::" + done <<< "$SPECS" + [ "$failed" -eq 0 ] || { + echo "::error::Spec path ownership check failed. A spec declares files owned by an ADR it does not cite." + exit 1 + } + + - name: Check spec scope (advisory) + if: steps.specs.outputs.count != '0' + env: + SPECS: ${{ steps.specs.outputs.files }} + run: | + set -euo pipefail + # Advisory by design - the cap is uncalibrated. It runs here rather + # than in the agent job specifically so its output lands on the PR the + # reviewer is reading, instead of in a step summary nobody opens. + while IFS= read -r spec; do + [ -n "$spec" ] || continue + echo "::group::check-spec-scope $spec" + SPEC_FILE="$spec" node scripts/ai-sdlc/check-spec-scope.mjs || true + echo "::endgroup::" + done <<< "$SPECS" From ea1b699797a300658aa7eec5866352c6e5cdba71 Mon Sep 17 00:00:00 2001 From: coder-abt Date: Tue, 11 Aug 2026 09:55:56 +0500 Subject: [PATCH 2/3] ci(ai-sdlc): address review findings on the PR spec-guard workflow Fixes real gaps found in review: catch renamed-and-modified specs by adding R to the ownership/scope file-discovery diff-filter (AM alone silently skipped them), disable credential persistence on the PR checkout, fail closed instead of fail open when file discovery errors, run the advisory scope check even when the hard ownership gate fails, correct the ownership-failure error text to match what the script actually checks, surface (rather than silently swallow) a scope-check launch failure, and fix a transposed commit SHA in a header comment. Assisted-by: claude-opus-5 (agent) --- .github/workflows/ai-sdlc-guards.yml | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ai-sdlc-guards.yml b/.github/workflows/ai-sdlc-guards.yml index 50cfbd49..4ce1d6a8 100644 --- a/.github/workflows/ai-sdlc-guards.yml +++ b/.github/workflows/ai-sdlc-guards.yml @@ -8,7 +8,7 @@ name: AI-SDLC Guards # - the spec ratification PR (Gate 1) did not re-run ownership on the version a # human edits and merges; # - PR #283, the delivery for issue #269, was written by hand and therefore -# passed through none of them, as did 13b3f6b and c0af5ec. +# passed through none of them, as did 13bf6b3 and c0af5ec. # # The guardrails were attached to the machine path; the delivery went down the # human path. This puts the spec-side guards on the PR, where the change is. @@ -41,6 +41,7 @@ jobs: uses: actions/checkout@v4 with: fetch-depth: 0 + persist-credentials: false - name: Collect spec files changed by this PR id: specs @@ -49,13 +50,16 @@ jobs: HEAD_SHA: ${{ github.event.pull_request.head.sha }} run: | set -euo pipefail - # Added or modified only. A deleted spec has nothing to verify, and - # feeding a deleted path to the guards would fail on the read rather - # than on the rule, which is a misleading way to go red. - files=$(git diff --name-only --diff-filter=AM "$BASE_SHA" "$HEAD_SHA" -- 'docs/specs/*.md' || true) + # Added, modified, or renamed only. A deleted spec has nothing to + # verify, and feeding a deleted path to the guards would fail on the + # read rather than on the rule, which is a misleading way to go red. + # R is included because a rename that also edits content (e.g. a + # spec renumbered alongside a wording fix) would otherwise be + # invisible to AM and skip both guards silently. + files=$(git diff --name-only --diff-filter=AMR "$BASE_SHA" "$HEAD_SHA" -- 'docs/specs/*.md') if [ -z "$files" ]; then echo "count=0" >> "$GITHUB_OUTPUT" - echo "No added or modified spec files; nothing to check." + echo "No added, modified, or renamed spec files; nothing to check." exit 0 fi echo "count=$(printf '%s\n' "$files" | wc -l)" >> "$GITHUB_OUTPUT" @@ -84,12 +88,12 @@ jobs: echo "::endgroup::" done <<< "$SPECS" [ "$failed" -eq 0 ] || { - echo "::error::Spec path ownership check failed. A spec declares files owned by an ADR it does not cite." + echo "::error::Spec path ownership check failed. A spec declares a new path under a component name that docs/adr/README.md attributes to a different repo. See the group log above for the specific path(s)." exit 1 } - name: Check spec scope (advisory) - if: steps.specs.outputs.count != '0' + if: ${{ !cancelled() && steps.specs.outputs.count != '0' }} env: SPECS: ${{ steps.specs.outputs.files }} run: | @@ -100,6 +104,8 @@ jobs: while IFS= read -r spec; do [ -n "$spec" ] || continue echo "::group::check-spec-scope $spec" - SPEC_FILE="$spec" node scripts/ai-sdlc/check-spec-scope.mjs || true + if ! SPEC_FILE="$spec" node scripts/ai-sdlc/check-spec-scope.mjs; then + echo "::warning::check-spec-scope crashed for $spec instead of reporting advisory output - this is a launch/script failure, not a scope finding" + fi echo "::endgroup::" done <<< "$SPECS" From 53b854784868d0a12a8636836f70957b0c437614 Mon Sep 17 00:00:00 2001 From: coder-abt Date: Tue, 11 Aug 2026 10:02:57 +0500 Subject: [PATCH 3/3] ci(ai-sdlc): pin Node.js version to match the rest of CI Every other workflow that runs scripts/ai-sdlc/*.mjs (spec-agent.yml, impl-agent.yml, ci.yml) pins Node 22 via actions/setup-node. This workflow ran the same scripts on whatever Node ubuntu-latest happens to ship, an unpinned dependency the rest of CI already avoids. Assisted-by: claude-opus-5 (agent) --- .github/workflows/ai-sdlc-guards.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/ai-sdlc-guards.yml b/.github/workflows/ai-sdlc-guards.yml index 4ce1d6a8..0a705c43 100644 --- a/.github/workflows/ai-sdlc-guards.yml +++ b/.github/workflows/ai-sdlc-guards.yml @@ -43,6 +43,11 @@ jobs: fetch-depth: 0 persist-credentials: false + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: '22' + - name: Collect spec files changed by this PR id: specs env: