[dshakes]

Two problems, both hit during today's PRs.

1. Re-auditing was unreachable

The audits fire on opened/reopened/labeled and deliberately not on synchronize — so a fix pushed in response to a finding was never re-audited, and the posted verdict kept describing the first commit.

The documented escape hatch ("add the agent:audit label") doesn't work once the PR already carries it: re-adding a label a PR already has fires no labeled event, so it silently does nothing. On #207 the only way to get a verdict on the fixed code was to remove the label and add it back.

synchronize is now a trigger, but only when the PR already carries agent:audit:

situation	behavior	credits
PR opens	one audit	1×
push, no label	no audit	none — unchanged
add agent:audit	audit	1×
push, label present	re-audits	opt-in

A plain PR still gets exactly one audit on open, so the fix loop doesn't re-spend credits. Labelling one means "keep auditing this as I fix it" — which makes the label meaningful rather than one-shot.

2. The status read as "never ran"

The Classifier's domain:* labels land seconds after open, and each one triggers this workflow, which correctly skips. That skipped run is newer than the real audit, so gh pr checks shows skipping while the audit had already succeeded.

I read that as "the auditors don't run for this change class" and said so — wrong, and the kind of wrong that gets a working review gate quietly written off. GitHub can't filter label names in on:, so the skipped run is unavoidable; both headers now explain the display, and the job comment says which labels may spend credits.

sdlc-design-review is untouched — its types: [labeled] + domain:ui gate is the intended routing pattern, not this bug.

Verification

actionlint clean. The behavior table above is being verified on this PR, which carries the change.

[github-actions]

🔎 Codex cross-audit (agent:audit)

No Blocking findings.

I don’t see correctness or security regressions introduced by this PR. The new synchronize trigger is gated on the existing agent:audit label in both workflows, and non-agent:audit label events still skip before spending model credits.

Validation performed:

• Reviewed git diff origin/master...HEAD
• Checked both modified workflows with line numbers
• Ran git diff --check origin/master...HEAD with no whitespace/syntax issues reported

Residual note: pushes without agent:audit will still create skipped workflow runs because synchronize is now a trigger, so check summaries may remain noisy. That matches the PR’s documented tradeoff rather than a blocker.

[github-actions]

🔷 Gemini cross-audit (agent:audit-gemini)

🔷 Gemini Audit Verdict: CONDITIONAL PASS (Blocking Issues Identified)

While the updated GHA triggering logic is syntactically correct and safely handles the opt-in synchronize (re-audit on push) behavior, there are two important issues—one high-priority race condition and one medium-priority usability issue—that should be addressed before merging.

---

1. ⚠️ High Priority: Stale Audits on synchronize via /merge Race Condition (Correctness)

• The Problem: On a synchronize (push) event, GitHub fires the webhook immediately, but the background generation of the pre-merged commit (refs/pull/$PR/merge) is asynchronous and often delayed.
• The Impact: Both sdlc-audit-gemini.yml (which fetches refs/pull/$PR/merge) and sdlc-audit.yml (which checks out /merge) will frequently fetch/checkout the stale merge commit from before the push. The audit will analyze outdated code, potentially flagging already-fixed bugs and ignoring the new changes.
• The Remedy:
  • In sdlc-audit-gemini.yml, fetch refs/pull/$PR/head instead of refs/pull/$PR/merge. The head ref is updated synchronously by GitHub and is guaranteed to be current. The three-dot diff git diff $BASE...FETCH_HEAD will correctly and robustly capture all changes in the PR without relying on the stale merge ref:

    git fetch --quiet origin "refs/pull/$PR/head"
    git diff "$BASE"...FETCH_HEAD > pr-diff.patch

  • In sdlc-audit.yml, check out refs/pull/${{ github.event.pull_request.number }}/head rather than /merge to ensure the Codex audit runs on the up-to-date commit.

---

2. ⚠️ Medium Priority: Missing API Key Check in sdlc-audit.yml (Usability)

• The Problem: Unlike sdlc-audit-gemini.yml which has a dedicated Check for GEMINI_API_KEY step and cleanly skips if the key is missing, sdlc-audit.yml lacks any check for the presence of OPENAI_API_KEY.
• The Impact: For pull requests opened from forks, secrets (including OPENAI_API_KEY) are not exposed. The Codex action step will run with an empty key, fail, and cause the status check of all fork PRs to turn red, harming contributor experience.
• The Remedy: Implement an API key presence check in sdlc-audit.yml mirroring the one in sdlc-audit-gemini.yml, and gate subsequent steps on steps.key.outputs.present == 'true':

  - name: Check for OPENAI_API_KEY
    id: key
    env:
      OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
    run: |
      if [ -n "$OPENAI_API_KEY" ]; then
        echo "present=true" >>"$GITHUB_OUTPUT"
      else
        echo "present=false" >>"$GITHUB_OUTPUT"
        echo "OPENAI_API_KEY not set — skipping the Codex cross-audit."
      fi

---

3. 🔍 Code & Logic Validation (Pass)

• GHA Syntax & Short-Circuiting: The new YAML conditional expression under if is robust:

  github.actor != 'dependabot[bot]' && (
    github.event.action == 'opened' ||
    github.event.action == 'reopened' ||
    (github.event.action == 'labeled' && github.event.label.name == 'agent:audit') ||
    (github.event.action == 'synchronize' &&
     contains(github.event.pull_request.labels.*.name, 'agent:audit'))
  )

  • During non-labeling events (like opened or synchronize), short-circuiting correctly prevents undefined access errors on github.event.label.name.
  • The map expression pull_request.labels.*.name is syntactically valid and safely evaluates to an empty array if no labels are present.
• No Double-Spending on Other Labels: If the PR is already labeled agent:audit and another label (e.g., domain:foo) is added, the action is labeled but the added label is not agent:audit. The job correctly skips, preventing unnecessary credit spend.

[github-actions]

🔎 Codex cross-audit (agent:audit)

No Blocking findings.

I found no correctness, security, or missed-edge regressions in the PR diff. The new synchronize trigger is gated behind agent:audit on both workflows, while opened/reopened behavior and non-agent:audit label skips remain consistent with the existing design.

Verification: inspected git diff origin/master...HEAD and ran git diff --check origin/master...HEAD; no whitespace/config errors reported. actionlint is not installed in this environment, so I could not run a full workflow linter.

[github-actions]

🔷 Gemini cross-audit (agent:audit-gemini)

🔴 Blocking Issues

1. Lack of Concurrency Control (High Risk of Costly & Redundant Runs)

• Issue: Neither .github/workflows/sdlc-audit-gemini.yml nor .github/workflows/sdlc-audit.yml defines a concurrency block.
• Impact: With the newly added synchronize trigger, every push to a PR carrying the agent:audit label will immediately spawn concurrent audit workflows. If a developer pushes multiple commits in rapid succession, multiple duplicate instances of the Gemini/Codex audit workflows will run in parallel, wasting API credits and spamming duplicate comments.
• Resolution: Add a concurrency group with cancel-in-progress: true in both workflows:

  concurrency:
    group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
    cancel-in-progress: true

2. Duplicate Auditing During Auto-Fix Loops (Extreme API Credit Waste)

• Issue: When a PR carrying agent:audit fails the Claude reviewer, the auto-fix loop (sdlc-fix.yml) is triggered. Each round of the loop pushes a commit, which fires synchronize.
• Impact: Under the proposed changes, each intermediate auto-fix commit pushed by the Builder will trigger a full, fresh Gemini and Codex audit. This is highly wasteful as intermediate commits are not final states.
• Resolution: Exclude audits on synchronize when the PR is undergoing automated fixes by checking that the PR does not carry agent:needs-fix or agent:fixing labels:

  (github.event.action == 'synchronize' &&
   contains(github.event.pull_request.labels.*.name, 'agent:audit') &&
   !contains(github.event.pull_request.labels.*.name, 'agent:needs-fix') &&
   !contains(github.event.pull_request.labels.*.name, 'agent:fixing'))

---

⚠️ Recommended Improvements & Edge Cases

1. Potential Checkout Failures on synchronize due to Stale Merge Refs

• Issue: Both workflows fetch/checkout the merge ref (refs/pull/$PR/merge) to construct the PR diff.
• Impact: On synchronize (push) events, GitHub fires the webhook immediately, but the merge commit is calculated asynchronously. If the calculation is slightly delayed or fails (e.g., due to merge conflicts), fetching or checking out the merge ref will fail, breaking the workflow.
• Resolution: For synchronize events, fetch the PR head branch (refs/pull/$PR/head) and run a triple-dot diff against the base branch, which is robust and immediate.

2. PR Comment Timeline Clutter

• Issue: Both workflows use gh pr comment to post audit reports.
• Impact: Repeated push events will result in dozens of repetitive comments from agent:audit-gemini and agent:audit, cluttering the PR timeline.
• Resolution: Update existing comments instead of posting new ones, or document that agent:audit should be removed during active developer iterations.

[github-actions]

🔎 Codex cross-audit (agent:audit)

No Blocking findings.

I did not find correctness, security, or missed-edge-case regressions in the PR diff. The new synchronize trigger is still gated by agent:audit in both workflows, and the label expression is scoped so ordinary pushes should create skipped jobs rather than spending audit credits.

Residual note: these workflows will now produce skipped runs on unlabeled synchronize events, similar to the already-documented skipped runs from non-audit labels. That could be noisy in check summaries, but I did not find evidence these audit jobs are required branch-protection checks.

Validation: parsed both workflow YAML files successfully. actionlint is not installed in this environment, so I could not run the GitHub Actions semantic linter.

[github-actions]

🔷 Gemini cross-audit (agent:audit-gemini)

I have audited the changes in pr-diff.patch for correctness, security, and edge cases. Below is the assessment.

---

🟢 Status: Approved (No Blocking Issues)

The changes are correct, secure, and idiomatic. They successfully implement an opt-in re-audit loop for both the Gemini and Codex auditors while minimizing token/credit spend.

---

🔍 Detailed Audit Findings

1. Logic & Correctness (Pass)

• Conditional Synchronization Trigger: The addition of synchronize to the pull_request triggers is correctly gated by the check:
  contains(github.event.pull_request.labels.*.name, 'agent:audit').
• Array-to-String Check: In GitHub Actions expression syntax, github.event.pull_request.labels.*.name correctly transforms the array of label objects into an array of strings. The contains function safely checks this array for the 'agent:audit' label.
• Precise Label Targeting: On a labeled event, the check (github.event.action == 'labeled' && github.event.label.name == 'agent:audit') is used instead of a generic contains check on all labels. This ensures that adding any other label (such as domain:*) does not trigger an audit run even if agent:audit is already present. This successfully addresses the "skipped run spam" concern described in the header comments.

2. Security (Pass)

• Least Privilege: Both workflows maintain minimal permissions (contents: read and pull-requests: write), which limits risk.
• Safe Checkouts on Untrusted PRs:
  • sdlc-audit-gemini.yml continues to safely check out the trusted base branch (github.event.pull_request.base.sha) and fetch the PR diff as raw data, completely neutralizing any attempt by a malicious PR to steer or exploit the workflow runner or API key.
  • sdlc-audit.yml runs with sandbox: read-only, protecting the workflow against untrusted code execution.
• Fork Protection: The secrets (GEMINI_API_KEY and OPENAI_API_KEY) remain secure as they are not passed to workflows triggered by pull requests from fork repositories.

3. Edge Cases & Reliability (Pass)

• Dependabot Exclusion: Standard exclusion github.actor != 'dependabot[bot]' is preserved to prevent failures on automated dependency updates.
• Clean Skips: If a PR has no labels, github.event.pull_request.labels is an empty array [] (not null), ensuring that the GHA expression evaluates safely without throwing syntax or evaluation errors.

[dshakes]

Verified on this PR's own CI

Same-repo PRs run the head branch's workflow, so this change tested itself. Codex audit runs, in order:

sha	event	agent:audit?	result
fa32c8f	opened	—	success	audit on open (always worked)
fa32c8f	labeled domain:infra	—	skipped	← the run that masked the real one
0feeef6	synchronize	no	skipped	cost control intact, no credits
0feeef6	labeled agent:audit	yes	success	label triggers
1d31568	synchronize	yes	success	← the fix: re-audits on push

The last row is the behaviour that did not exist before. Under the old config that push was ignored entirely and the posted verdict kept describing an older commit — which is why #207 needed the remove-then-re-add dance to get a verdict on fixed code.

Row 2 is the display bug, reproduced live: a skipped label-triggered run lands newer than the successful audit, so gh pr checks reads skipping while the audit had already passed. GitHub can't filter label names in on:, so both headers now explain it.