Publish durable completed-batch audit receipts#203
Conversation
|
Warning Review limit reached
Next review available in: 24 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughCompleted-batch audits now publish verified durable GitHub comments through a shared Ruby receipt module. Coordinator ownership, compact final output, manifest-bound replay, marker validation, shared audit integration, and comprehensive tests were added or tightened. ChangesCompleted-batch receipt flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Coordinator
participant CompletedBatchAuditReceipt
participant GitHubAPI
participant DurableComment
Coordinator->>CompletedBatchAuditReceipt: publish completed-batch receipt and targets
CompletedBatchAuditReceipt->>GitHubAPI: validate target and write permission
CompletedBatchAuditReceipt->>DurableComment: POST canonical receipt
DurableComment-->>CompletedBatchAuditReceipt: return comment ID and metadata
CompletedBatchAuditReceipt->>GitHubAPI: read back and verify exact comment
Coordinator->>CompletedBatchAuditReceipt: replay compact reference
CompletedBatchAuditReceipt->>GitHubAPI: fetch manifest-bound comment ID
GitHubAPI-->>CompletedBatchAuditReceipt: return comment bindings and body
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Review: Publish durable completed-batch audit receiptsOverviewThis PR adds a
Overall this is a very thoroughly tested, defensively written module (fail-closed everywhere, extensive adversarial-style tests for bot actors, timestamp edits, mismatched URLs, enterprise ports, etc.). A couple of things stood out on closer read: Potential issues
Nits
SecurityNo command-injection risk found — Test coverageVery strong — the new test file covers the failure matrix (wrong author, edited comment, mismatched URL, bot actors on user/permission/comment payloads, ambiguous POST outcomes, enterprise port canonicalization, empty manifests, malformed external blockers) exhaustively via a fake |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/post-merge-audit/bin/completed-batch-audit-receipt`:
- Around line 169-180: Update the replay path around select_verified_anchor so
it does not reselect the anchor using the current GitHub actor’s permissions or
lock state. Resolve and validate the canonical target from the durable receipt’s
author/manifest binding, then keep the existing target equality check and
comment verification flow using that replay-stable anchor.
- Around line 812-828: Update the rescue handling around the generic exception
path so INVALID_MARKER_BLOCKER is added only for parser or batch-binding
failures. Map permission, API, unknown POST outcome, and readback exception
classes to their exact operational blockers, and build blockers and final_status
from those mapped values so mutation-recovery actions remain visible; preserve
the existing mutation_state and comment_id fields for PostOutcomeUnknownError
and PostReadbackError.
- Around line 206-208: Update the author capture in parse_reference to accept
underscores alongside the existing alphanumeric and hyphen characters, matching
the unchanged authenticated GitHub login emitted by compact_reference while
preserving the rest of the reference validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 036db5aa-6b42-45fc-a9e0-dcd188d2b95d
📒 Files selected for processing (10)
CHANGELOG.mdbin/validateskills/post-merge-audit/SKILL.mdskills/post-merge-audit/bin/completed-batch-audit-receiptskills/post-merge-audit/bin/completed-batch-audit-receipt-test.rbskills/post-merge-audit/bin/post-merge-audit-policy-test.rbskills/pr-batch/SKILL.mdskills/pr-batch/bin/goal-completion-contract-test.rbworkflows/post-merge-audit.mdworkflows/pr-processing.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f52b61e3a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Review: Publish durable completed-batch audit receipts (PR 203) Overview
Code quality / correctness
Portability concern (see inline comment) Minor / lower-confidence notes
Security Overall: solid, well-tested change. The one thing worth addressing (or consciously accepting) before merge is the new cross-skill file dependency noted above. |
Review: Publish durable completed-batch audit receipts (#203)OverviewThis PR replaces the old approach of emitting the full Code quality / correctness
Security
Performance
Test coverage
Overall this is a well-scoped, carefully fail-closed implementation with strong test coverage. Only a trivial dead-code nit found (left as an inline comment); no correctness or security blockers identified. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f35f8a733b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/post-merge-audit/bin/completed-batch-audit-receipt`:
- Line 23: Move timeout parsing from the top-level GH_TIMEOUT_SECONDS assignment
into the run command’s structured error boundary. In the completed-batch audit
execution flow, parse COMPLETED_BATCH_AUDIT_GH_TIMEOUT_SECONDS and validate that
it is a positive integer before any API operation; ensure invalid or
non-positive values produce run’s JSON failure response instead of failing
during module load.
- Around line 396-405: Update canonical_comment_body to preserve the supplied
marker/receipt wrapper byte-for-byte: remove the newline-appending
transformation in canonical_marker, or reject input that is not already
canonical. Ensure marker_state validation and the returned comment use the
unchanged marker.
In `@skills/post-merge-audit/bin/completed-batch-audit-receipt-test.rb`:
- Around line 931-971: Remove the fragile elapsed wall-clock assertion and its
associated timing setup from
test_post_timeout_is_bounded_and_reaps_child_before_delayed_side_effect. Keep
the child reaping check, delayed-side-effect absence check, mutation-state
assertions, and POST call count unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 60c7b37d-a262-40ed-a455-3380587b2862
📒 Files selected for processing (5)
skills/post-merge-audit/SKILL.mdskills/post-merge-audit/bin/completed-batch-audit-receiptskills/post-merge-audit/bin/completed-batch-audit-receipt-test.rbskills/pr-batch/SKILL.mdskills/pr-batch/bin/goal-completion-contract-test.rb
🚧 Files skipped from review as they are similar to previous changes (1)
- skills/pr-batch/bin/goal-completion-contract-test.rb
Review: Publish durable completed-batch audit receipts (#203)OverviewThis PR changes how the Code quality / correctness
Security
Test coverage
Performance
Overall this looks like a solid, well-tested change that matches its stated goal (durable, verifiable audit receipts instead of raw marker text in chat). Only minor/non-blocking suggestions left inline. |
Summary
Validation
Closes #184
Summary by CodeRabbit