amadan
console: on a claim console an absent record leaves the delete to the claim (re-review finding 10)
F7 answered 200 deleted for any delete whose check found no record, before it considered host claims. A claim retirement deletes the record and then parks while obligations remain, so a retry in that interval was told the delete was done while the host and app stayed blocked, and it skipped the claim's own pending answer. The absent-record shortcut now applies only with host claims off. With claims on, the absent path returns to the claim retirement flow as before, so a retry gets the operation's pending answer (202 with its phase) and its cleanup.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 files changed,
+64
−1
internal/console/apiinstanceimport.go+8 −1internal/console/importdelete_claim_test.go+56 −0
CI failed — run details
diff --git a/internal/console/apiinstanceimport.go b/internal/console/apiinstanceimport.go| index 3d5f72a3a..693525c43 100644 |
| --- a/internal/console/apiinstanceimport.go |
| +++ b/internal/console/apiinstanceimport.go |
| @@ -895,7 +895,8 @@ func (s *Server) abortAnswered(w http.ResponseWriter, r *http.Request, sqid, app |
| // - committed, a --from seed, an unseeded record, or a record that will |
| // not decode: the ordinary delete, conditional on the etag read here |
| // (deleteRead), answered as finished. |
| -// - no record at all: finished, with nothing deleted (final review F7). |
| +// - no record at all: finished, with nothing deleted (final review F7), |
| +// except on a claim console, where the claim's retirement answers. |
| // |
| // It fails closed (preflight M15): a store read that fails, a seed id that |
| // cannot name a key (M8) and a finish that fails answer main's delete |
| @@ -921,6 +922,12 @@ func (s *Server) importDeleteRule(w http.ResponseWriter, r *http.Request, acct * |
| sqid := acct.Sqid |
| rec, etag, err := s.readDeletedRecord(r.Context(), sqid, app, host) |
| if errors.Is(err, os.ErrNotExist) { |
| + // With host claims on, a record can be gone while its retirement |
| + // is not (parking, with obligations left): the claim's own flow |
| + // answers that, pending or done (re-review finding 10). |
| + if s.cfg.HostClaim { |
| + return false, false |
| + } |
| return true, true |
| } |
| if err != nil { |
diff --git a/internal/console/importdelete_claim_test.go b/internal/console/importdelete_claim_test.go| new file mode 100644 |
| index 000000000..16117d917 |
| --- /dev/null |
| +++ b/internal/console/importdelete_claim_test.go |
| @@ -0,0 +1,56 @@ |
| +package console |
| + |
| +import ( |
| + "context" |
| + "encoding/json" |
| + "errors" |
| + "net/http" |
| + "os" |
| + "testing" |
| + |
| + "github.com/carlosframework/platform/internal/hostclaim" |
| + "github.com/carlosframework/platform/internal/provision" |
| +) |
| + |
| +// Re-review finding 10: on a claim console the delete's retirement can |
| +// delete the record and then park while obligations remain. A retried |
| +// delete in that interval must get the claim flow's answer (its pending |
| +// operation and phase), never a 200 that says the delete is done. |
| +func TestClaimDeleteRetryWhileParkingIsNotDone(t *testing.T) { |
| + const host = "acme.go.tito.io" |
| + cx := newClaimCase(t) |
| + cx.instanceWorld(t) |
| + cx.instance(t, `{"host":"`+host+`","region":"ie"}`) |
| + url := cx.ts.URL + "/api/cli/apps/bab/titogo/instances/" + host |
| + status, _, out := rawReq(t, http.MethodDelete, url, cx.bearer, "") |
| + if status != http.StatusAccepted { |
| + t.Fatalf("delete = %d %s, want 202", status, out) |
| + } |
| + op := claimReplyField(t, out, "operation_id") |
| + parking := func() bool { |
| + obs, err := hostclaim.Read(context.Background(), cx.rel, host) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + _, gone := provision.GetRecord(context.Background(), cx.rel, "bab", "titogo", host) |
| + return !obs.Absent && obs.Claim.Op.Kind == hostclaim.KindParking && len(obs.Claim.Obligations) > 0 && errors.Is(gone, os.ErrNotExist) |
| + } |
| + for i := 0; i < 6 && !parking(); i++ { |
| + cx.ackClaim(t, host, op) |
| + claimWorker("StartClaimRecoveryPass")(t, cx) |
| + } |
| + if !parking() { |
| + t.Fatal("the claim world did not reach parking with obligations left") |
| + } |
| + status, _, out = rawReq(t, http.MethodDelete, url, cx.bearer, "") |
| + if status == http.StatusOK { |
| + t.Fatalf("a retry while the claim parks answered %d %s: the delete is not done", status, out) |
| + } |
| + if status != http.StatusAccepted { |
| + t.Fatalf("retry = %d %s, want the claim flow's 202", status, out) |
| + } |
| + var reply map[string]any |
| + if err := json.Unmarshal([]byte(out), &reply); err != nil || reply["operation_id"] != op || reply["phase"] != hostclaim.KindParking { |
| + t.Fatalf("retry answered %s, want operation %s at %s", out, op, hostclaim.KindParking) |
| + } |
| +} |