feat(review): add v2.2 submission descriptors - #2233
Merged
Conversation
Alan-TheGentleman
added
size:exception
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAdds v2.2 capabilities and a status-v4 contract for provider-issued submission descriptors. The CLI generates, validates, and executes bound correction and targeted-validation submissions. Journeys and acceptance tests cover external working directories, rejection handling, and terminal approval. ChangesSubmission descriptor contract
Transition and execution flow
Journey and validation coverage
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
There was a problem hiding this comment.
Pull request overview
This PR extends the review-integration protocol surface by publishing capabilities v2.2 and a native Git status v4 schema, and adds provider-bound submission descriptors for the two external collect handoffs (correction plan + targeted validation) using the existing review.finalize operation. It also strengthens replay/forgery resistance by requiring exact argv execution against provider-issued bindings and supports running from arbitrary working directories via opaque repository context.
Changes:
- Add v2.2 capabilities + status/v4 contracts (schema + fixture) and update conformance/pinning tests.
- Emit one-slot, provider-bound submission descriptors on the two collect transitions and validate they are correctly bound / safe.
- Add
review finalizesupport for executing provider-issued descriptor routes (including opaque repository context resolution) plus bench and binary acceptance coverage.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| internal/reviewtransaction/repository_locator.go | Allow opaque repository-context validation for correction-required flows (including targeted-validation binding). |
| internal/cli/review_next_transition.go | Add submission descriptor structs + emit provider-bound argv templates for correction/validation collect steps. |
| internal/cli/review_status_contract.go | Introduce status/v4 identity handling and enforce descriptor binding/placement/shape validation. |
| internal/cli/review_facade.go | Add descriptor-bound review finalize flags + exact-argv verification + opaque-root resolution for arbitrary CWD execution. |
| internal/cli/review_operation_contract.go | Register new finalize flags in operation metadata. |
| internal/cli/review_submission_descriptor_test.go | New end-to-end tests for descriptor binding, replay safety, and “single substituted value” execution behavior. |
| internal/cli/review_next_transition_test.go | Add v4 next_transition schema validation helper and refactor shared schema compilation to handle v1/v2 roots. |
| internal/cli/review_correction_plan_request_test.go | Update to validate next_transition against the published v4 schema. |
| internal/cli/review_provider_artifact_contract_test.go | Add pinning for v2.1 artifacts and extend strict schema checks for new v2.2/v4 assets. |
| internal/cli/review_capabilities.go | Publish capabilities protocol 2.2 and add provider_submission_descriptors feature. |
| internal/cli/review_capabilities_test.go | Validate v2.2 fixture/schema conformance and ensure v2.1 artifacts remain readable. |
| internal/cli/review_binary_acceptance_test.go | Prove built binary can execute descriptor routes from an unrelated CWD without leaking paths. |
| contracts/review-integration/v2/schemas/status-v4.schema.json | New published status/v4 schema including submission descriptor constraints. |
| contracts/review-integration/v2/schemas/capabilities-v2.2.schema.json | New published capabilities v2.2 schema (protocol 2.2 surface). |
| contracts/review-integration/v2/fixtures/capabilities-v2.2.fixture.json | New v2.2 conformance fixture. |
| bench/runner.go | Allow journey steps to invoke the product from arbitrary directories. |
| bench/journeys_wave1.go | Exercise descriptor-based correction + validation completion in the wave journey flow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 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 `@bench/journeys_wave1.go`:
- Around line 497-499: Update the validation around descriptor.ArgumentTokens in
the submission descriptor flow to verify that at least five tokens exist before
accessing ArgumentTokens[4]. Preserve the existing placeholders and
--request-hash= checks, returning the current diagnostic error instead of
allowing a short token list to panic.
In `@contracts/review-integration/v2/schemas/capabilities-v2.2.schema.json`:
- Line 95: Update the shared schema definition for feature names and change the
items within the requires property to reference it instead of accepting
arbitrary non-empty strings. Preserve uniqueItems and ensure all listed feature
names, including compact_v2_authority, are covered by the enum.
In `@internal/cli/review_binary_acceptance_test.go`:
- Around line 405-417: Consolidate the binary execution helpers by making
runReviewBinaryAt delegate to the shared implementation rather than duplicating
command setup and validation. Preserve its working-directory behavior and
stdout-only return contract, and update runReviewBinaryStreams to delegate to
runReviewBinaryStreamsAt using an empty directory as proposed.
In `@internal/cli/review_facade.go`:
- Around line 2132-2145: Remove the redundant outer err check after the
repository-root branches. In the submissionBindingProvided branch, handle and
return the error from resolveOpaqueReviewRepositoryRoot immediately; keep the
existing wrapped error return in the SnapshotBuilder ResolveRepositoryRoot path,
ensuring each branch handles its own error exactly once.
- Around line 2533-2536: Validate submission.Value.SubstitutionLocation against
the length of the copied ArgumentTokens slice before indexing expected in the
argument comparison flow. If the slot is negative or outside the slice, return
the existing preflight failure result instead of panicking; preserve the current
replacement and DeepEqual behavior for valid slots.
- Around line 2491-2494: Refactor validateReviewFinalizeSubmission to accept a
small struct grouping the descriptor-related fields, including expectedRevision,
targetIdentity, requestHash, and repositoryContext, instead of passing these
positional values individually. Update every caller to construct and pass the
struct, while preserving the existing validation behavior and other arguments.
In `@internal/cli/review_next_transition.go`:
- Around line 465-507: Extract the six shared leading tokens from
reviewCorrectionPlanSubmission and reviewTargetedValidationSubmission into a
helper, and define reviewSubmissionValueLocation from that prefix length. Use
the helper in both builders, replace their literal substitution location 6 with
reviewSubmissionValueLocation, and update ReviewTransitionSubmission.Validate to
slice using the same constant while preserving the existing token order and
validation behavior.
In `@internal/cli/review_status_contract.go`:
- Around line 542-568: Update the repository-context validation in both the
correction-plan and targeted-validation branches of the surrounding transition
validator: compare input.submissionRepositoryContext() against the authoritative
input.Arguments["repository-context"] value, rather than using that self-derived
context to build reviewCorrectionPlanSubmission or
reviewTargetedValidationSubmission. Preserve the existing authority checks and
reject descriptors whose context differs from the bound capture argument.
In `@internal/cli/review_submission_descriptor_test.go`:
- Around line 88-98: Extend the validation rejection table covering
ReviewTransitionSubmission.Validate with token-order cases using
swapSubmissionTokens: include a general token swap and a swap that moves the
placeholder token out of index 6. Use validationPath as the submitted value and
assert both reordered descriptors are rejected, matching the existing
correction-table coverage.
In `@internal/reviewtransaction/repository_locator.go`:
- Around line 334-337: Update the validation flow around
BuildTargetedValidationRequest to return its underlying err unchanged when
request construction fails. Only return the “review repository context is stale
or has no live matching authority” error when the request succeeds but
correction.CorrectionTargetIdentity differs from binding.TargetIdentity.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d14c756e-9482-4b00-bc68-1ae9cc0a5db7
📒 Files selected for processing (17)
bench/journeys_wave1.gobench/runner.gocontracts/review-integration/v2/fixtures/capabilities-v2.2.fixture.jsoncontracts/review-integration/v2/schemas/capabilities-v2.2.schema.jsoncontracts/review-integration/v2/schemas/status-v4.schema.jsoninternal/cli/review_binary_acceptance_test.gointernal/cli/review_capabilities.gointernal/cli/review_capabilities_test.gointernal/cli/review_correction_plan_request_test.gointernal/cli/review_facade.gointernal/cli/review_next_transition.gointernal/cli/review_next_transition_test.gointernal/cli/review_operation_contract.gointernal/cli/review_provider_artifact_contract_test.gointernal/cli/review_status_contract.gointernal/cli/review_submission_descriptor_test.gointernal/reviewtransaction/repository_locator.go
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.
Suppressed comments (1)
internal/cli/review_facade.go:2107
- When executing a provider-issued submission descriptor (i.e. using --repository-context / --expected-revision / --target / --request-hash), --lineage is effectively required as part of the repository-context binding. Currently the preflight allows descriptor mode with an empty lineage, but ResolveReviewRepositoryContext will always reject it via validateReviewRepositoryContextBinding, causing a later/less specific failure.
if submissionBindingProvided && (!negotiated || *contract != ReviewIntegrationContractV2 || strings.TrimSpace(*expectedSubmissionRevision) == "" ||
strings.TrimSpace(*targetIdentity) == "" || strings.TrimSpace(*requestHash) == "" || strings.TrimSpace(*repositoryContext) == "") {
return reviewPreflightError(errors.New("review finalize submission descriptors require the complete v2 revision, target, request hash, and repository context binding; refresh with gentle-ai review status --next-transition"))
This was referenced Aug 2, 2026
20 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 Linked Issue
Closes #1611
🏷️ PR Type
type:feature— Versioned protocol capability📝 Summary
review.finalize.📂 Changes
🧪 Test Plan
✅ Contributor Checklist
type:*labelsize:exceptionSize exception
size:exceptionrequested for 1,425 changed lines (+1,376/-49 across 19 files).This single vertical v2.2 contract change is 1,425 changed lines (+1,376/-49 across 19 files). The schema/fixture/status surface, runtime, immutable-artifact compatibility checks, arbitrary-CWD binary coverage, and black-box benchmark flows must land together to prevent protocol skew. No dependency, workflow, release, or unrelated product changes are included.
Summary by CodeRabbit
New Features
Bug Fixes