Skip to content

Fix Claude review diagnostics and scheduled usage accounting - #945

Closed
seathatflowsinourveins wants to merge 1 commit into
mainfrom
codex/ns2604-claude-review-fixes-20261009
Closed

seathatflowsinourveins wants to merge 1 commit into
mainfrom
codex/ns2604-claude-review-fixes-20261009

Conversation

@seathatflowsinourveins

Copy link
Copy Markdown
Owner

Scope

Zero-cost Claude errors currently abort accounting in jq before the summary explains the failure, and scheduled review jobs that reject large diffs still consume the daily review allowance. Preserve a fixed error class in both review summaries, skip diffs above 250,000 bytes before allocating a matrix slot, and count retained usage artifacts instead of started jobs.

  • Base commit: 3c01bdddc66896f8e36f9f21d452710808a9557e.
  • Lane: lane:foundation.
  • Owned paths: .github/workflows/claude-pr-review.yml, .github/workflows/claude-pr-toolkit-review.yml, their two decision records, and the existing two workflow test modules. manifests/evidence.json refreshes only the hashes and byte counts of those six changed files.
  • Schedule: 7,22,37,52 * * * * UTC. Both workflows set top-level cache-mode: none.
  • Both decision records describe the verified immutable OIDC subject, including owner and repository IDs.
  • harness-audit.yml is outside this slice and remains with its separate owner in Bound the harness audit and move it to claude-code-action v1.0.247 #892.

When is_error is true, total_cost_usd is zero and modelUsage is {}, error_class is the fixed value zero_cost_error_without_model_usage. The summary records that class and the bounds still fail; raw error text is excluded and no review is published. Other records carry a null class. Toolkit incomplete accounting remains incomplete, with null cost and turns and any actual assistant tokens retained only as a lower bound.

The resolver measures the REST diff response in bytes and skips oversized heads before either of its two slots is allocated. Exactly 250,000 bytes qualifies; a manual dispatch may use paths to narrow a larger diff. Exact-head binding and the later merge-base git diff guard still enforce the final limit. Daily accounting reads every page of nonexpired usage artifacts from this workflow's schedule and dispatch runs on main, requires the artifact name to match its run, and counts each name once. The existing $5.50 reservation per usage record remains; this is not a sum of billed costs. Jobs without usage records consume no room. API failures remain fail-closed.

SOTA sources

Evidence-class table

Claim Evidence class Command / receipt
Zero-cost error diagnostic in both workflows, oversized-head preflight, usage-only allowance, cron and cache contracts fail before their changes and pass after synthetic, local_integration Existing workflow step tests execute the actual workflow shell with synthetic execution files and a local GitHub CLI stand-in; fail-before rounds recorded, then 135 tests pass
Existing workflow policy, security coverage, hardening and spend checks remain satisfied local_integration 184 related tests run, 183 pass / 1 skip
Both decision records match the configured immutable subject format and IDs source_review Native non-secret OIDC customization and repository metadata API responses; official OIDC reference
Workflow syntax and local security analysis pass local_integration actionlint 1.17.0, rc0; zizmor 1.30.1 with CI flags, rc0, no findings. Local zizmor reports offline fallback; hosted online CI is a separate check
Publication integrity passes local_integration FULL python3 scripts/validate.py, rc0: 70 components, 11,283 hashed files, 4 profiles, 239 receipts
Changed workflow behavior on a hosted model run untested No model session, federation exchange or paid review was started by this change; the draft needs the designated Claude read

Local commands run

python3 -m unittest tests.test_claude_pr_review_workflow tests.test_claude_pr_toolkit_review_workflow
# rc0; 135 tests passed

python3 -m unittest tests.test_workflow_policy tests.test_workflow_security tests.test_workflow_security_coverage tests.test_workflow_hardening tests.test_claude_spend_window tests.test_claude_spend_scan
# rc0; 184 tests run, 1 skipped

actionlint -color
# rc0; installed 1.17.0, CI command

zizmor --no-config --no-ignores --persona regular --strict-collection .
# rc0; installed 1.30.1, no findings; local offline fallback explicitly reported

python3 scripts/validate.py
# FULL rc0 after refreshing only the six changed-file hash/size entries

git diff --check
# rc0

Nine new or updated regression contracts were observed failing before implementation: two zero-cost errors, two oversized-diff boundary/starvation cases, two usage-versus-job accounting cases, cron and two cache contracts. The pagination/expired-name test additionally protects the new accounting boundary. The pre-commit secret gate passed with pinned gitleaks 8.30.1; native repository pre-push hooks run the committed host-name/history scan and registry checks without a bypass.

Decision record

  • docs/decisions/2026-10-08-claude-actions-pr-review.md, “Review workflow corrections (2026-10-09, CC dispatch item 7)”.
  • docs/decisions/2026-10-09-claude-actions-pr-toolkit-review.md, “Zero-cost error diagnostic (2026-10-09, CC dispatch item 7)”.

Host evidence

No host receipt or platform-status change.

Checklist

  • Changed GitHub Actions retain their full commit pins and version comments.
  • Changed workflows retain top-level permissions: {} and only their existing job grants.
  • No secrets printed, logged or committed; no new secret required.
  • No new paid hosting, subscription or billing surface.
  • Peer-owned files and worktrees preserved.

Draft for the command center's designated Claude read. Landing and any third-workflow cache change remain with their owners.

@seathatflowsinourveins seathatflowsinourveins added the lane:foundation Foundation lane: Claude/Codex setup, hosts, memory, RAG, research, workers label Oct 9, 2026
@socket-security

Copy link
Copy Markdown

Dependency limit exceeded — report not shown.

This pull request scan exceeded the 10,000-dependency limit applied to this scan, so the results are incomplete and may be inaccurate. To avoid reporting false positives, Socket has not posted a report.

Upgrade your plan to raise the dependency limit and get complete reports, or view the partial scan in the dashboard.

Socket is always free for open source. If this is a non-commercial open source project, contact us to request a free Team account.

@seathatflowsinourveins

Copy link
Copy Markdown
Owner Author

Closing as superseded, with the owner's OK (CC, 2026-10-11).

It modifies .github/workflows/claude-pr-review.yml and claude-pr-toolkit-review.yml. #981 deleted both, so the merge would be a modify/delete conflict.

Claude review inside GitHub was retired by #981 (8bffa2c, docs/decisions/2026-10-10-retire-in-github-claude-review.md). The Anthropic API refuses the action's entry point on Console keys (CC correction #141; the action never had a successful run). AI reviews now run locally, and GitHub enforces them as required checks. The local route is #953 (the review worker).

Nothing is lost: this PR can be reopened, and any model-free part of it can come back as a new PR under the local-review route. Evidence: the CC's PR triage (research-cc-role-20261008/pr-triage-20261011.md, class CLOSE).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lane:foundation Foundation lane: Claude/Codex setup, hosts, memory, RAG, research, workers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant