Skip to content

fix: reconcile absent ASCII Box claims #1874

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
shunkakinoki wants to merge 1 commit into
base: main
Choose a base branch
from
+110 −65
Open

fix: reconcile absent ASCII Box claims #1874

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
35 changes: 19 additions & 16 deletions docs/providers/ascii-box.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,13 +120,17 @@ waiting because of a timeout, cancellation, or operation lookup failure, it
durably records the exact operation ID and its claim binding before returning
the error. The binding covers the original provider scope, Box ID, creation
timestamp, and repository owner. This records acceptance, not completion. A later
`crabbox stop` first reads that same operation. Pending, processing, or blocked
operations retain the claim without normal Box lookups, SSH teardown, or another
deletion request. Only a matching operation that explicitly reports `completed`
with a valid completion timestamp can proceed to `box info` not-found and
complete inventory absence checks. Crabbox repeats these operation and absence
checks inside the actual release fence before removing the claim; an earlier
lookup during lease resolution does not authorize finalization.
`crabbox stop` first checks the exact Box identity. If the Box remains observable,
Crabbox validates any recorded operation before proceeding. Pending, processing,
or blocked operations retain the claim without SSH teardown or another deletion
request. A matching operation that reports `completed` also retains the claim
while the Box remains observable because those native results are inconsistent.
If `box info` instead reports a recognized not-found and complete
`box list --all` inventory also omits the exact Box ID, Crabbox reconciles the
unchanged local claim without reading a stale operation or repeating native
deletion. Crabbox repeats these checks inside the actual release fence before
removing the claim; an earlier lookup during lease resolution does not authorize
finalization.

If Crabbox finishes waiting for native deletion but final inventory confirmation
fails or is canceled, it durably records that completed deletion in the
Expand All @@ -140,15 +144,14 @@ retain the claim.
Completion records from earlier unreleased builds that proved only native
request acceptance are rejected, not upgraded into operation-completion evidence.

Not-found and empty inventory alone are not deletion-completion evidence: ASCII
can hide a Box while its deletion operation is still pending or blocked. Claims
without either Crabbox's completion record or its bound accepted-operation
reference stay retained, including external deletions, native commands interrupted
before valid acceptance was observed, and a process termination before the record
was durably written. Missing, malformed, changed, or incomplete record bindings
are rejected. There is no automatic adoption of an external deletion receipt,
replacement of a recorded operation, or conversion of an old claim into
completed-deletion authority.
Exact not-found plus complete inventory absence is independent deletion evidence;
it authorizes only removal of the unchanged local claim and never another native
mutation. Failed or partial inventory, an observable matching ID, a replacement
identity, cancellation, and missing or malformed native responses retain the
claim. Missing, malformed, changed, or incomplete operation-record bindings are
still rejected when the Box remains observable. There is no automatic adoption
of an external deletion receipt, replacement of a recorded operation, or
conversion of an old claim into completed-deletion authority.

Raw IDs, provider names, and legacy claims without the full ownership binding
remain inspectable but cannot authorize reuse or deletion. Missing or changed
Expand Down
42 changes: 21 additions & 21 deletions internal/providers/asciibox/ownership.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,29 +317,29 @@ func exactBoxForRelease(ctx context.Context, client api, expected boxData) (boxD
if err := ctx.Err(); err != nil {
return boxData{}, false, err
}
if expected.deletionOperationID != "" {
operation, err := client.GetDeletionOperation(ctx, expected.ID, expected.deletionOperationID)
if ctxErr := ctx.Err(); ctxErr != nil {
return boxData{}, false, ctxErr
}
if err != nil {
return boxData{}, false, fmt.Errorf("ascii-box deletion operation lookup; retaining claim: %w", err)
}
if err := validateBoxDeletionOperation(operation, expected.ID, expected.deletionOperationID); err != nil {
return boxData{}, false, err
}
if operation.Status != "completed" {
return boxData{}, false, exit(2, "ascii-box deletion operation %s is %s; retaining claim", operation.ID, operation.Status)
}
// Recheck the recorded operation inside the release fence; a reference
// or an earlier resolution read is not completion authority.
expected.deletionCompleted = true
}
fresh, err := client.GetBox(ctx, expected.ID)
if ctxErr := ctx.Err(); ctxErr != nil {
return boxData{}, false, ctxErr
}
if err == nil {
if expected.deletionOperationID != "" {
operation, operationErr := client.GetDeletionOperation(ctx, expected.ID, expected.deletionOperationID)
if ctxErr := ctx.Err(); ctxErr != nil {
return boxData{}, false, ctxErr
}
if operationErr != nil {
return boxData{}, false, fmt.Errorf("ascii-box deletion operation lookup; retaining claim: %w", operationErr)
}
if err := validateBoxDeletionOperation(operation, expected.ID, expected.deletionOperationID); err != nil {
return boxData{}, false, err
}
if operation.Status != "completed" {
return boxData{}, false, exit(2, "ascii-box deletion operation %s is %s; retaining claim", operation.ID, operation.Status)
}
// Recheck the recorded operation inside the release fence; a reference
// or an earlier resolution read is not completion authority.
expected.deletionCompleted = true
}
if expected.deletionCompleted {
return boxData{}, false, exit(2, "ascii-box %s is still observable after recorded deletion completion; retaining claim", expected.ID)
}
Expand All @@ -348,9 +348,9 @@ func exactBoxForRelease(ctx context.Context, client api, expected boxData) (boxD
if !isNotFound(err) {
return boxData{}, false, fmt.Errorf("ascii-box ownership lookup; retaining claim: %w", err)
}
if !expected.deletionCompleted {
return boxData{}, false, exit(2, "ascii-box %s has no completed native deletion witness; absence alone cannot prove deletion completion; retaining claim", expected.ID)
}
// Exact native not-found plus a complete inventory absence is independent
// deletion authority. It permits stale-claim reconciliation without
// repeating a mutation or waiting on an obsolete operation record.
boxes, listErr := client.ListBoxes(ctx, true)
if ctxErr := ctx.Err(); ctxErr != nil {
return boxData{}, false, ctxErr
Expand Down
98 changes: 70 additions & 28 deletions internal/providers/asciibox/ownership_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,17 +233,21 @@ func TestReleasePendingReferenceRejectsChangedBinding(t *testing.T) {
}
}

func TestReleasePendingReferenceRechecksCompletionInsideFence(t *testing.T) {
func TestReleasePendingReferenceRechecksObservableBoxInsideFence(t *testing.T) {
b, f, claim := pendingDeletionFixture(t)
lookups := 0
f.getHook = func(string) (boxData, error) {
lookups++
if lookups == 1 {
return boxData{}, errors.New("404 not found")
}
return f.box, nil
}
f.listHook = func() ([]boxData, error) { return []boxData{}, nil }
reads := 0
f.deletionHook = func(targetID, operationID string) (boxDeletionOperation, error) {
reads++
op := boxDeletionOperation{ID: operationID, Kind: "box", TargetID: targetID, Status: "completed", CompletedAt: "2026-09-02T09:00:00Z"}
if reads > 1 {
op.Status = "blocked"
op.CompletedAt = ""
}
return op, nil
return boxDeletionOperation{ID: operationID, Kind: "box", TargetID: targetID, Status: "blocked"}, nil
}
lease, err := b.Resolve(context.Background(), ResolveRequest{ID: claim.LeaseID, ReleaseOnly: true})
if err != nil {
Expand All @@ -253,7 +257,7 @@ func TestReleasePendingReferenceRechecksCompletionInsideFence(t *testing.T) {
if err := b.ReleaseLease(context.Background(), ReleaseLeaseRequest{Lease: lease, GuardedRemoteCleanup: func(context.Context, LeaseTarget) { teardown = true }}); err == nil {
t.Fatal("earlier completion read replaced release-fence verification")
}
if reads != 2 || teardown || len(f.deletedIDs) != 0 || len(f.prepareIDs) != 0 {
if lookups != 2 || reads != 1 || teardown || len(f.deletedIDs) != 0 || len(f.prepareIDs) != 0 {
t.Fatalf("unsafe pending retry: reads=%d teardown=%t deleted=%v prepared=%v", reads, teardown, f.deletedIDs, f.prepareIDs)
}
assertClaimRetained(t, claim)
Expand All @@ -263,6 +267,7 @@ func TestReleasePendingReferenceRejectsUncertainOperation(t *testing.T) {
for _, failure := range []string{"operation", "target", "kind", "completion timestamp", "lookup", "canceled"} {
t.Run(failure, func(t *testing.T) {
b, f, claim := pendingDeletionFixture(t)
f.deleted = false
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
f.deletionHook = func(targetID, operationID string) (boxDeletionOperation, error) {
Expand All @@ -283,10 +288,6 @@ func TestReleasePendingReferenceRejectsUncertainOperation(t *testing.T) {
}
return op, nil
}
f.getHook = func(string) (boxData, error) {
t.Fatal("uncertain operation reached Box lookup")
return boxData{}, nil
}
if _, err := b.Resolve(ctx, ResolveRequest{ID: claim.LeaseID, ReleaseOnly: true}); err == nil {
t.Fatal("uncertain operation authorized cleanup")
}
Expand All @@ -311,14 +312,15 @@ func TestReleasePendingDeletionSurvivesTimeoutAndRetries(t *testing.T) {
}
pending := assertPendingDeletionRetained(t, claim, testDeletionID)
runner.outcomes["deletion"] = []commandOutcome{deletionOutcome(testDeletionID, claim.CloudID, "box", "blocked")}
runner.outcomes["info"] = []commandOutcome{info}
commandCount := len(runner.commands)
if _, err := b.Resolve(context.Background(), ResolveRequest{ID: claim.LeaseID, ReleaseOnly: true}); err == nil {
t.Fatal("blocked operation was treated as completed")
}
assertClaimRetained(t, pending)
for _, command := range runner.commands[commandCount:] {
if strings.Contains(command, " info ") || strings.Contains(command, " list ") || strings.Contains(command, " stop ") || strings.Contains(command, " delete ") || strings.Contains(command, " ssh ") {
t.Fatalf("pending retry used Box lookup or mutation: %s", command)
if strings.Contains(command, " list ") || strings.Contains(command, " stop ") || strings.Contains(command, " delete ") || strings.Contains(command, " ssh ") {
t.Fatalf("pending retry used inventory or mutation: %s", command)
}
}
runner.outcomes["deletion"] = []commandOutcome{deletionOutcome(testDeletionID, claim.CloudID, "box", "completed"), deletionOutcome(testDeletionID, claim.CloudID, "box", "completed")}
Expand Down Expand Up @@ -396,7 +398,7 @@ func TestReleaseRejectsUnownedAndChangedClaims(t *testing.T) {
}

func TestReleaseRetainsUncertainResources(t *testing.T) {
for _, name := range []string{"wrong ID", "missing timestamp", "changed timestamp", "lookup 404", "lookup failure", "failed deletion", "bad confirmation", "cancelled", "replacement in inventory"} {
for _, name := range []string{"wrong ID", "missing timestamp", "changed timestamp", "lookup 404", "lookup 404 with failed inventory", "lookup 404 with replacement", "lookup failure", "failed deletion", "bad confirmation", "cancelled", "replacement in inventory"} {
t.Run(name, func(t *testing.T) {
b, f, claim, lease := ownedFixture(t)
ctx, cancel := context.WithCancel(context.Background())
Expand All @@ -413,6 +415,14 @@ func TestReleaseRetainsUncertainResources(t *testing.T) {
f.box.CreatedAt = "2026-08-30T12:00:01Z"
case "lookup 404":
f.getHook = func(string) (boxData, error) { return boxData{}, fmt.Errorf("404 not found") }
case "lookup 404 with failed inventory":
f.getHook = func(string) (boxData, error) { return boxData{}, fmt.Errorf("404 not found") }
f.listHook = func() ([]boxData, error) { return nil, fmt.Errorf("partial inventory") }
case "lookup 404 with replacement":
f.getHook = func(string) (boxData, error) { return boxData{}, fmt.Errorf("404 not found") }
replacement := f.box
replacement.CreatedAt = "2026-08-30T12:00:01Z"
f.listHook = func() ([]boxData, error) { return []boxData{replacement}, nil }
case "lookup failure":
f.getHook = func(string) (boxData, error) { return boxData{}, fmt.Errorf("network unavailable") }
case "failed deletion":
Expand Down Expand Up @@ -442,21 +452,24 @@ func TestReleaseRetainsUncertainResources(t *testing.T) {
}
}

func TestReleaseRetainsAbsentBoxWithoutCompletedDeletion(t *testing.T) {
b, f, claim, lease := ownedFixture(t)
func TestReleaseReconcilesAbsentBoxWithCompleteInventory(t *testing.T) {
b, f, claim, _ := ownedFixture(t)
f.getHook = func(string) (boxData, error) { return boxData{}, fmt.Errorf("404 not found") }
f.listHook = func() ([]boxData, error) { return []boxData{}, nil }

if _, err := b.Resolve(context.Background(), ResolveRequest{ID: claim.LeaseID, ReleaseOnly: true}); err == nil {
t.Fatal("release-only resolution accepted absence without deletion completion")
lease, err := b.Resolve(context.Background(), ResolveRequest{ID: claim.LeaseID, ReleaseOnly: true})
if err != nil {
t.Fatal(err)
}
if err := b.ReleaseLease(context.Background(), ReleaseLeaseRequest{Lease: lease}); err == nil {
t.Fatal("release accepted absence without deletion completion")
if err := b.ReleaseLease(context.Background(), ReleaseLeaseRequest{Lease: lease}); err != nil {
t.Fatal(err)
}
if len(f.deletedIDs) != 0 {
t.Fatalf("deleted=%v, want no unverified native deletion", f.deletedIDs)
t.Fatalf("deleted=%v, want claim-only reconciliation", f.deletedIDs)
}
if _, exists, err := core.ReadLeaseClaimWithPresence(claim.LeaseID); err != nil || exists {
t.Fatalf("claim remains after complete absence confirmation: exists=%t err=%v", exists, err)
}
assertClaimRetained(t, claim)
}

func TestReleaseRejectsCancellationDuringAbsenceCheck(t *testing.T) {
Expand Down Expand Up @@ -570,7 +583,7 @@ func TestReleaseRejectsChangedCompletionWitness(t *testing.T) {
if err := b.ReleaseLease(context.Background(), ReleaseLeaseRequest{Lease: lease}); err == nil {
t.Fatal("stale release snapshot accepted replacement claim")
}
if name != "revision" {
if name != "missing" && name != "revision" {
if _, err := b.Resolve(context.Background(), ResolveRequest{ID: claim.LeaseID, ReleaseOnly: true}); err == nil {
t.Fatal("changed claim retained deletion authority")
}
Expand Down Expand Up @@ -616,7 +629,7 @@ func TestReleaseCompletedWitnessRetainsUncertainResource(t *testing.T) {
}
}

func TestReleaseRetainsHiddenPendingDeletion(t *testing.T) {
func TestReleaseReconcilesHiddenPendingDeletionAfterCompleteAbsence(t *testing.T) {
b, f, claim, lease := ownedFixture(t)
f.releaseHook = func(string) error {
f.deleted = true
Expand All @@ -626,10 +639,37 @@ func TestReleaseRetainsHiddenPendingDeletion(t *testing.T) {
t.Fatal("pending native deletion succeeded")
}
assertClaimRetained(t, claim)
if err := b.ReleaseLease(context.Background(), ReleaseLeaseRequest{Lease: lease}); err == nil {
t.Fatal("hidden pending deletion was mistaken for completed deletion")
if err := b.ReleaseLease(context.Background(), ReleaseLeaseRequest{Lease: lease}); err != nil {
t.Fatal(err)
}
if len(f.deletedIDs) != 0 {
t.Fatalf("claim reconciliation repeated native deletion: %v", f.deletedIDs)
}
if _, exists, err := core.ReadLeaseClaimWithPresence(claim.LeaseID); err != nil || exists {
t.Fatalf("claim remains after complete absence confirmation: exists=%t err=%v", exists, err)
}
}

func TestReleaseReconcilesAbsentBoxWithoutReadingStaleOperation(t *testing.T) {
b, f, claim := pendingDeletionFixture(t)
f.deletionHook = func(string, string) (boxDeletionOperation, error) {
t.Fatal("complete absence reached stale deletion operation")
return boxDeletionOperation{}, nil
}

lease, err := b.Resolve(context.Background(), ResolveRequest{ID: claim.LeaseID, ReleaseOnly: true})
if err != nil {
t.Fatal(err)
}
if err := b.ReleaseLease(context.Background(), ReleaseLeaseRequest{Lease: lease}); err != nil {
t.Fatal(err)
}
if len(f.deletedIDs) != 0 {
t.Fatalf("claim reconciliation repeated native deletion: %v", f.deletedIDs)
}
if _, exists, err := core.ReadLeaseClaimWithPresence(claim.LeaseID); err != nil || exists {
t.Fatalf("claim remains after complete absence confirmation: exists=%t err=%v", exists, err)
}
assertClaimRetained(t, claim)
}

func TestBoxNotFoundRejectsIdentityErrors(t *testing.T) {
Expand Down Expand Up @@ -791,6 +831,7 @@ func TestAcquireRollbackRetainsDeletionEvidence(t *testing.T) {
var retained LeaseClaim
if completion == "pending" {
retained = assertPendingDeletionRetained(t, published, testDeletionID)
f.deleted = false
f.deletionHook = func(targetID, operationID string) (boxDeletionOperation, error) {
return boxDeletionOperation{ID: operationID, Kind: "box", TargetID: targetID, Status: "blocked"}, nil
}
Expand All @@ -801,6 +842,7 @@ func TestAcquireRollbackRetainsDeletionEvidence(t *testing.T) {
f.deletionHook = func(targetID, operationID string) (boxDeletionOperation, error) {
return boxDeletionOperation{ID: operationID, Kind: "box", TargetID: targetID, Status: "completed", CompletedAt: "2026-09-02T09:00:00Z"}, nil
}
f.deleted = true
} else {
retained = assertCompletedDeletionRetained(t, published)
}
Expand Down
Loading