Skip to content

Clarify missing per-run AI credit rejection evidence - #68738

Closed
pelikhan wants to merge 2 commits into
mainfrom
pelikhan-credit-rejection-evidence
Closed

pelikhan wants to merge 2 commits into
mainfrom
pelikhan-credit-rejection-evidence

Conversation

@pelikhan

Copy link
Copy Markdown
Collaborator

Why

Daily Max AI Credits Test run 38556022048 recorded AWF usage of 1.021966 credits and a genuine Codex credit-limit HTTP 403, but its artifact contained no qualifying structured ai_credits_limit_exceeded event. The verifier's generic failure obscured this runtime-evidence mismatch and could be misread as an enforcement failure.

Approach

  • Bind verification to the current repository, run ID and run attempt; reject explicitly conflicting event identities.
  • Inspect token-tracker audit files alongside existing firewall candidates. Report collected record counts, invalid rejection fields, last authoritative usage and top-level Codex terminal credit errors separately.
  • Preserve fail-closed proof: only the exact structured AWF rejection at cap 1 can pass. Generic 403s, failures, report text, usage totals and quoted tool output never substitute for proof.
  • Add a normalized captured diagnostic fixture and update the workflow explanation and generated lock. Raw private logs are not committed.

This improves diagnostics rather than claiming to repair or prove enforcement. Accounting, pricing, caps, retry budgets, permissions and sandbox controls are unchanged. The missing boundary remains AWF structured stdout rejection evidence versus the persisted artifact; the producer/collection cause and secondary cache-miss classification are not fixed here.

Validation

  • 45 focused Node regressions passed, including malformed evidence, wrong limits/runs/attempts/repositories, quoted tool output, generic 403s, evidence ordering and the captured genuine rejection remaining unverified.
  • Original artifact replay now explicitly reports 37 records in 2 firewall files, usage of 1.021966 at/above cap 1, and a separate diagnostic-only Codex rejection while retaining an unverified verdict.
  • make recompile: 344/344 workflows compiled; normal lock changes only body-hash metadata.
  • make agent-report-progress passed formatting, build, schema freshness and workflow drift. The impacted Go selector skipped tests because no Go files changed.
  • Retained single-reviewer source/normal-lock security review passed with no confirmed vulnerabilities.

Remaining gaps

The installed older compiler's initial dry-run emitted no lock due to schema incompatibility. The current repository compiler emitted a diagnostic dry-run lock and passed source/model checks and shellcheck, but the dry-run gate failed on the existing gh-aw-detection experimental notice. No experimental opt-in was granted; optional scanners were not run. No workflow was dispatched or rerun, and hosted behavior for this change remains unverified.

Bind credit verification to the current repository, run and attempt. Distinguish absent structured AWF rejection events from collected usage and diagnostic-only Codex errors while preserving fail-closed proof requirements.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@pelikhan
pelikhan marked this pull request as ready for review October 14, 2026 15:35
Copilot AI balanced review requested due to automatic review settings October 14, 2026 15:35
@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft.

Design Decision Gate 🏗️

Outcome: reported_no_action

The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible.

Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.

Generated from Design Decision Gate 🏗️ · opus50 · 13.7 AIC · ⌖ 40 AIC · ◇ 37.3 AIC · ⊞ 1.8K · ◷

🏗️ ADR gate enforced by Design Decision Gate 🏗️

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

✅ PR Code Quality Reviewer completed the code quality review.

🔎 Code quality review by PR Code Quality Reviewer

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

✅ Test Quality Sentinel completed test quality analysis.

Test Quality Sentinel

Outcome: reported_no_action

The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible.

Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.

Generated from Test Quality Sentinel · sonnet50 · 13.6 AIC · ⌖ 14.8 AIC · ◇ 6.42 AIC · ⊞ 11.3K · ◷

🧪 Test quality analysis by Test Quality Sentinel

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

✅ Ponytail Reviewer completed successfully!

Ponytail Reviewer

Outcome: reported_no_action

The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible.

Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.

Generated from Ponytail Reviewer · gpt56 · 8.74 AIC · ⌖ 6.76 AIC · ⊞ 14.3K · ◷

Generated by Ponytail Reviewer for #68738

Copilot AI left a comment

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.

🟡 Changes recommended

The verifier requires aw_info.json, but the verification job downloads only an artifact that does not contain it.

1 open finding
What changed in this PR

Improves fail-closed diagnostics when structured AI-credit rejection evidence is absent.

Changes:

  • Binds evidence to repository, run, and attempt.
  • Reports usage and Codex 403 diagnostics separately.
  • Adds regression coverage and a normalized run fixture.

The linked run audit corroborates the 1.021966-credit diagnostic. No security regression was found, but artifact wiring is incomplete.

File Description
.github/​workflows/​daily-max-ai-credits-test.md Documents verification behavior.
.github/​workflows/​daily-max-ai-credits-test.lock.yml Updates generated body metadata.
.github/​scripts/​fixtures/​max-ai-credits-rejection-38556022048.json Adds normalized diagnostic evidence.
.github/​scripts/​check-max-ai-credits.test.cjs Expands verifier regressions.
.github/​scripts/​check-max-ai-credits.cjs Adds identity checks and diagnostics.

🧠 Review effort: Balanced

@github-actions

Copy link
Copy Markdown
Contributor

Comment Memory

Peek at saved memory (pr-code-quality-reviewer)
reviewed_at: 2026-10-14T15:38:19.434+00:00
review_event: REQUEST_CHANGES
top_themes:
  - optional diagnostic sidecars breaking proof paths
  - artifact identity binding adding false-negative failure modes
files_reviewed:
  - .github/scripts/check-max-ai-credits.cjs
  - .github/scripts/check-max-ai-credits.test.cjs
  - .github/workflows/daily-max-ai-credits-test.md
comment_count: 3

Note

This comment is managed by comment memory.

Expand the saved memory block to view or edit the persistent context for this thread.
Edit only the text inside the backtick fences; workflow metadata and the footer are regenerated automatically.

Learn more about comment memory

🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 34.2 AIC · ⌖ 6.1 AIC · ⊞ 21.3K · ◷
Comment /review to run again

@github-actions github-actions Bot left a comment

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.

Request changes

This diagnostic hardening also makes optional sidecar files part of the proof path, so a real structured rejection can now be reported as a failed run for unrelated artifact-corruption reasons.

Blocking themes
  • token-tracker-audit.jsonl is now treated as a mandatory, fail-fast input even though it is only useful for failure diagnostics.
  • token-usage.jsonl is parsed before we know whether we need diagnostics, so a truncated usage sidecar can override a valid pass.
  • aw_info.json is now required before any firewall evidence is considered, which adds another artifact-packaging dependency ahead of the actual rejection proof.

🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 34.2 AIC · ⌖ 6.1 AIC · ⊞ 21.3K
Comment /review to run again

@github-actions github-actions Bot left a comment

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.

Skills-Based Review 🧠

Applied /diagnosing-bugs (root-cause diagnostics) and /tdd (regression coverage). Solid change — approving with one minor, non-blocking note.

📋 Key Themes & Highlights

Key Themes

  • Root cause correctly scoped: the PR explicitly frames this as improving diagnostics, not "fixing" enforcement — it doesn't overclaim, and the fail-closed contract (only an exact structured ai_credits_limit_exceeded event at cap 1 passes) is preserved throughout the new code paths.
  • Regression coverage is thorough: 45 tests cover identity-mismatch rejection (wrong run/attempt/repo), malformed JSON in tracker/usage files, non-authoritative agent-stdio text (quoted logs, report_incomplete, tool output, wrong-cap 403s), and private-payload redaction in diagnostics. This is a textbook /tdd regression suite for a diagnostics change — ran locally, all 45 pass.
  • Captured fixture is well-documented: fixtures/max-ai-credits-rejection-38556022048.json has an explicit provenance field describing what was redacted/normalized from the real run, which is good practice for reproducing real incidents without leaking private artifact logs.
  • Minor: left one inline note on check-max-ai-credits.cjs:83 — a redundant re-check of entry?.event === "ai_credits_limit_exceeded" that's already guaranteed by the loop's continue filter a few lines above. Purely cosmetic, doesn't affect behavior.

Positive Highlights

  • ✅ Identity binding (repository/run_id/run_attempt) correctly sources real fields that generate_aw_info.cjs populates — verified against the actual aw_info.json producer.
  • ✅ Diagnostics (file/record counts, usage totals, Codex terminal error text) are additive-only and never substitute for the structured rejection event — the fail-closed invariant holds.
  • ✅ Tests explicitly assert that private payload strings never leak into thrown diagnostic messages.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 152.3 AIC · ⌖ 14.6 AIC · ⊞ 10.4K
Comment /matt to run again

Download the separate current-run identity artifact and keep tracker and usage sidecars off the positive proof path. Cover source/lock packaging, malformed optional diagnostics and strict authoritative rejection evidence.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

Great work on the AI credit rejection evidence diagnostics! Solid tests and a clear description — this looks ready for review.

Generated by ✅ Contribution Check · copilot · auto · 44.5 AIC · ⌖ 0.613 AIC · ⊞ 9.3K · ◷

@pelikhan pelikhan closed this Oct 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants