Skip to content

fix(review): retain staged-equivalent receipts in status #2115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dnlrsls wants to merge 1 commit into
base: main
Choose a base branch
from
+198 −0
Open

fix(review): retain staged-equivalent receipts in status #2115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 88 additions & 0 deletions internal/cli/review_next_transition_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,94 @@ func TestValidatingEvidenceCollectionUnblocksFinalizeAndPreCommit(t *testing.T)
}
}

func TestNegotiatedPreCommitStatusRetainsApprovedStagedIntendedReceipt(t *testing.T) {
repo := initReviewCLIRepo(t)
for _, path := range []string{"first.txt", "second.txt"} {
if err := os.WriteFile(filepath.Join(repo, path), []byte("reviewed "+path+"\n"), 0o644); err != nil {
t.Fatal(err)
}
}
lineage := "next-transition-staged-intended"
var startedOutput bytes.Buffer
if err := RunReviewFacadeStart([]string{"--cwd", repo, "--lineage", lineage}, &startedOutput); err != nil {
t.Fatal(err)
}
var started ReviewFacadeStartResult
decodeStrictReviewJSON(t, startedOutput.Bytes(), &started)
store, err := reviewtransaction.CompactAuthoritativeStore(context.Background(), repo, lineage)
if err != nil {
t.Fatal(err)
}
record, err := store.Load()
if err != nil {
t.Fatal(err)
}
for order, lens := range record.State.SelectedLenses {
input := filepath.Join(t.TempDir(), fmt.Sprintf("result-%d.json", order))
if err := os.WriteFile(input, admittedReviewerPayloadForTest(t, repo, record, lens, order), 0o600); err != nil {
t.Fatal(err)
}
if err := RunReviewCaptureResult([]string{
"--cwd", repo, "--lineage", started.LineageID, "--target", record.State.InitialSnapshot.Identity,
"--lens", lens, "--order", fmt.Sprint(order), "--input", input,
}, &bytes.Buffer{}); err != nil {
t.Fatal(err)
}
}
if err := RunReviewFacadeFinalize([]string{"--cwd", repo, "--lineage", lineage, "--captured-results"}, &bytes.Buffer{}); err != nil {
t.Fatal(err)
}
record, err = store.Load()
if err != nil {
t.Fatal(err)
}
evidence := filepath.Join(t.TempDir(), "evidence.txt")
if err := os.WriteFile(evidence, []byte("verification passed\n"), 0o600); err != nil {
t.Fatal(err)
}
if err := RunReview([]string{
"capture-evidence", "--cwd", repo, "--lineage", lineage, "--target", record.State.CurrentSnapshot.Identity,
"--expected-revision", record.Revision, "--outcome", string(reviewtransaction.VerificationOutcomePassed), "--input", evidence,
}, &bytes.Buffer{}); err != nil {
t.Fatal(err)
}
if err := RunReviewFacadeFinalize([]string{"--cwd", repo, "--lineage", lineage, "--captured-evidence"}, &bytes.Buffer{}); err != nil {
t.Fatal(err)
}
runReviewCLIGit(t, repo, "add", "--", "first.txt", "second.txt")

status := func(selector ...string) ReviewTargetStatusResult {
t.Helper()
args := []string{
"status", "--contract", ReviewIntegrationContractV1, "--next-transition", "--cwd", repo,
"--projection", string(reviewtransaction.ProjectionStaged), "--gate", string(reviewtransaction.GatePreCommit),
}
args = append(args, selector...)
var output bytes.Buffer
if err := RunReview(args, &output); err != nil {
t.Fatalf("negotiated staged status: %v\n%s", err, output.String())
}
var result ReviewTargetStatusResult
decodeStrictReviewJSON(t, output.Bytes(), &result)
return result
}

explicit := status("--lineage", lineage)
unqualified := status()
for _, result := range []ReviewTargetStatusResult{explicit, unqualified} {
if result.Applicability != reviewtransaction.TargetApplicabilityCurrent ||
result.Action != reviewtransaction.TargetStatusActionValidate || result.Receipt.Status != ReviewReceiptPresent ||
result.Authority == nil || result.Authority.LineageID != lineage || result.NextTransition == nil ||
result.NextTransition.Execute == nil || result.NextTransition.Execute.Operation != "review.validate" {
t.Fatalf("approved staged receipt status = %#v", result)
}
}
if !reflect.DeepEqual(explicit.Authority, unqualified.Authority) || explicit.Receipt != unqualified.Receipt ||
explicit.NextTransition.Execute.Operation != unqualified.NextTransition.Execute.Operation {
t.Fatalf("explicit and unqualified staged status diverged: explicit %#v, unqualified %#v", explicit, unqualified)
}
}

func TestFinalizeNextTransitionBindsCorrectedCurrentSnapshot(t *testing.T) {
initialTarget := strings.Repeat("a", 64)
currentTarget := strings.Repeat("b", 64)
Expand Down
15 changes: 15 additions & 0 deletions internal/reviewtransaction/target_status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,21 @@ func assessTargetStatusSnapshot(ctx context.Context, repo string, request Target
} else if compactLiveTargetMatchesValidatedSnapshot(state, live, true) {
candidates = append(candidates, candidate)
continue
} else if state.State == StateApproved && candidate.receiptPublished && candidate.receiptCanonical &&
(state.CurrentSnapshot.Projection == "" || state.CurrentSnapshot.Projection == ProjectionWorkspace) && live.Projection == ProjectionStaged {
// PRE-COMMIT represents reviewed intended-untracked files in the index.
// Reuse its exact target assessment instead of weakening STATUS matching:
// it accepts only the identical candidate, base, and complete path digest.
assessment, assessmentErr := AssessCompactGateTarget(ctx, repo, state, NativeGateRequestInput{
Gate: GatePreCommit, LineageID: state.LineageID,
})
if assessmentErr != nil {
return targetStatusFailure(base, assessmentErr)
}
if assessment.Applicability == CompactGateTargetExact {
candidates = append(candidates, candidate)
Comment on lines +247 to +254

@coderabbitai coderabbitai Bot Jul 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

ast-grep outline internal/reviewtransaction/target_status.go --items all
rg -n -C 16 '^func AssessCompactGateTarget\b|^func targetStatusForCandidate\b|BuildStoredSnapshot|TargetIdentity' internal/reviewtransaction -g '*.go'

Repository: Gentleman-Programming/gentle-ai

Length of output: 50388

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the status flow and compact gate API without reading unrelated files.
wc -l internal/reviewtransaction/target_status.go internal/reviewtransaction/compact_native_gate.go internal/reviewtransaction/compact_state.go
sed -n '1,290p' internal/reviewtransaction/target_status.go
printf '\n--- compact_native_gate.go ---\n'
sed -n '1,260p' internal/reviewtransaction/compact_native_gate.go
printf '\n--- compact_state.go relevant --\n'
rg -n -C 10 'type NativeGateRequestInput|func AssessCompactGateTarget|TargetIdentity|InitialSnapshot|BuildStoredSnapshot|AssessTargetStatusWithSnapshot' internal/reviewtransaction/compact_native_gate.go internal/reviewtransaction/compact_state.go internal/reviewtransaction/target_status.go

Repository: Gentleman-Programming/gentle-ai

Length of output: 387

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- locate relevant files ---'
git ls-files internal/reviewtransaction | grep -E '(target_status|compact|native|gate|state)\.go$' | sed -n '1,120p'

printf '%s\n' '--- target_status.go top and relevant ranges ---'
sed -n '1,280p' internal/reviewtransaction/target_status.go

printf '%s\n' '--- search for NativeGateRequestInput and AssessCompactGateTarget ---'
rg -n -C 16 'type NativeGateRequestInput|func AssessCompactGateTarget|AssessCompactGateTarget\(' internal/reviewtransaction -g '*.go'

printf '%s\n' '--- search for snapshot builders/stores in target_status.go ---'
rg -n -C 8 'BuildStoredSnapshot|AssessTargetStatusWithSnapshot|targetStatusForCandidate|TargetIdentity|AssessCompactGateTarget' internal/reviewtransaction/target_status.go -g '*.go'

Repository: Gentleman-Programming/gentle-ai

Length of output: 50387

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- compact_gate.go ---'
sed -n '1,180p' internal/reviewtransaction/compact_gate.go

printf '%s\n' '--- gate context/build functions ---'
rg -n -C 12 'func buildCompactGateRequest|func buildCompactLifecycleSnapshot|type GateRequest|type Stage|func.*Staged' internal/reviewtransaction -g '*.go'

printf '%s\n' '--- compact.go relevant snapshot types/functions ---'
rg -n -C 10 'type CompactState|type CompactRecord|type Snapshot|func BuildStoredSnapshot|type SnapshotBuilder|TargetIdentity|CandidateTree|PathsDigest|CandidatePathDigest' internal/reviewtransaction/compact.go internal/reviewtransaction/target_status.go -g '*.go'

Repository: Gentleman-Programming/gentle-ai

Length of output: 50388

Bind PRE-COMMIT gate targeting to the status snapshot.

assessTargetStatusSnapshot uses live as the returned TargetIdentity, but AssessCompactGateTarget rebuilds the staged target from the live index via NativeGateRequestInput. A staged change between the initial snapshot and this gate can make the exact gate accept one snapshot while STATUS reports a newer status snapshot identity. Derive this assessment from the same live snapshot, or fail closed unless the assessed snapshot identity matches live.Identity, and add an interleaving test for that TOCTOU window.

🤖 Prompt for 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.

In `@internal/reviewtransaction/target_status.go` around lines 247 - 254, Update
the PRE-COMMIT targeting flow around AssessCompactGateTarget so assessment uses
the same live status snapshot represented by the returned TargetIdentity, rather
than rebuilding from the mutable live index; alternatively reject the candidate
unless the assessed snapshot identity equals live.Identity. Add an interleaving
test covering a staged change between the initial snapshot and gate assessment.

continue
}
}
if request.LineageID == "" && candidate.receiptPublished && (state.State == StateApproved || state.State == StateEscalated) {
if projectCompactTerminalHistory(state, live) == compactTerminalHistoryScopeChanged {
Expand Down
95 changes: 95 additions & 0 deletions internal/reviewtransaction/target_status_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,101 @@ func TestAssessTargetStatusDerivesReceiptTruthWithoutMutation(t *testing.T) {
}
}

func TestAssessTargetStatusRetainsApprovedReceiptForExactStagedIntendedTransition(t *testing.T) {
requireSnapshotGit(t)
tests := []struct {
name string
stage func(t *testing.T, repo string)
wantCurrent bool
}{
{
name: "all reviewed intended paths staged exactly",
stage: func(t *testing.T, repo string) {
gitSnapshot(t, repo, "add", "--", "tracked.txt", "first.txt", "second.txt")
},
wantCurrent: true,
},
{
name: "only a subset of intended paths staged",
stage: func(t *testing.T, repo string) {
gitSnapshot(t, repo, "add", "--", "tracked.txt", "first.txt")
},
},
{
name: "additional unreviewed staged path",
stage: func(t *testing.T, repo string) {
writeSnapshotFile(t, repo, "extra.txt", "not reviewed\n")
gitSnapshot(t, repo, "add", "--", "tracked.txt", "first.txt", "second.txt", "extra.txt")
},
},
{
name: "reviewed staged content changed",
stage: func(t *testing.T, repo string) {
writeSnapshotFile(t, repo, "first.txt", "changed after review\n")
gitSnapshot(t, repo, "add", "--", "tracked.txt", "first.txt", "second.txt")
},
},
{
name: "reviewed staged mode changed",
stage: func(t *testing.T, repo string) {
gitSnapshot(t, repo, "add", "--", "tracked.txt", "first.txt", "second.txt")
gitSnapshot(t, repo, "update-index", "--chmod=+x", "--", "first.txt")
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
repo := initSnapshotRepo(t)
writeSnapshotFile(t, repo, "tracked.txt", "reviewed tracked change\n")
for _, path := range []string{"first.txt", "second.txt"} {
writeSnapshotFile(t, repo, path, "reviewed "+path+"\n")
}
lineage := "status-staged-intended-" + strings.ReplaceAll(tt.name, " ", "-")
state, _, _ := approvedCompactCurrentChangesFixture(t, repo, lineage, []string{"first.txt", "second.txt"})
tt.stage(t, repo)
assessment, err := AssessCompactGateTarget(context.Background(), repo, state, NativeGateRequestInput{
Gate: GatePreCommit, LineageID: lineage,
})
if err != nil {
t.Fatal(err)
}

request := TargetStatusRequest{Target: Target{
Kind: TargetCurrentChanges, Projection: ProjectionStaged, IntendedUntracked: []string{},
}, LineageID: lineage}
explicit, err := AssessTargetStatus(context.Background(), repo, request)
if err != nil {
t.Fatal(err)
}
request.LineageID = ""
unqualified, err := AssessTargetStatus(context.Background(), repo, request)
if err != nil {
t.Fatal(err)
}

if tt.wantCurrent {
if assessment.Applicability != CompactGateTargetExact {
t.Fatalf("exact staged pre-commit assessment = %#v, expected %#v", assessment, state.CurrentSnapshot)
}
if explicit.Applicability != TargetApplicabilityCurrent || explicit.Action != TargetStatusActionValidate ||
explicit.State != StateApproved || explicit.ReceiptIdentity == "" || explicit.LineageID != lineage {
t.Fatalf("explicit staged status = %#v", explicit)
}
if unqualified.Applicability != TargetApplicabilityCurrent || unqualified.Action != TargetStatusActionValidate ||
unqualified.LineageID != explicit.LineageID || unqualified.ReceiptIdentity != explicit.ReceiptIdentity ||
unqualified.AuthorityTargetIdentity != state.CurrentSnapshot.Identity {
t.Fatalf("unqualified staged status = %#v, explicit %#v", unqualified, explicit)
}
return
}
if explicit.Applicability == TargetApplicabilityCurrent || explicit.Action == TargetStatusActionValidate ||
unqualified.Applicability == TargetApplicabilityCurrent || unqualified.Action == TargetStatusActionValidate {
t.Fatalf("inexact staged status became current: explicit %#v, unqualified %#v", explicit, unqualified)
}
})
}
}

func TestAssessTargetStatusClassifiesAllApplicabilityStates(t *testing.T) {
requireSnapshotGit(t)
request := TargetStatusRequest{Target: Target{Kind: TargetCurrentChanges, IntendedUntracked: []string{}}}
Expand Down
Loading