From b7c994d3cc82dc92c999542f19126a9c2fed03b3 Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 12:39:35 +0000 Subject: [PATCH 1/8] Delete only settled Sessions and confirm repeated owner deletion Session DELETE now follows the observed official lifecycle (SES-29/30). Under the Session lock that orders Turn and input admission, a Session with a queued, running or waiting Turn or a pending input reservation returns 409 conflict_error with the official message and nothing changes. The owner's repeated deletion returns the same 200 without writing; foreign, missing and malformed IDs keep the identical 404. Tests cover the HTTP/PostgreSQL state matrix with a no-write digest, deletion racing Turn and input admission on one row lock, a Worker cancel-then-delete flow and the pinned-SDK script. Tests that exercise hidden work under deletion markers from earlier releases now create those markers explicitly. --- contracts/agents-api/openapi.yaml | 16 +- services/agents-api/README.md | 22 +- services/agents-api/internal/api/errors.go | 5 +- .../agents-api/internal/api/errors_test.go | 11 + .../internal/api/session_deletion.go | 4 +- .../store/environment_admission_test.go | 5 +- .../store/environment_claim_worker_test.go | 5 +- .../store/environment_initial_input_test.go | 7 + .../store/environment_input_activity_test.go | 5 +- .../environment_input_settlement_test.go | 8 +- .../internal/store/environment_work_test.go | 6 +- .../agents-api/internal/store/export_test.go | 8 + .../store/prepared_dispatch_failure_test.go | 5 +- .../internal/store/session_artifacts_test.go | 6 +- .../internal/store/session_deletion.go | 52 +++- .../store/session_deletion_execution_test.go | 82 ++++- .../session_deletion_lifecycle_public_test.go | 230 ++++++++++++++ .../internal/store/session_deletion_test.go | 283 ++++++++++++++++-- .../internal/store/session_transaction.go | 16 +- .../store/subagent_identities_test.go | 5 +- .../internal/store/worker_input_race_test.go | 4 +- .../tests/official_session_delete.py | 28 +- 22 files changed, 751 insertions(+), 62 deletions(-) create mode 100644 services/agents-api/internal/store/session_deletion_lifecycle_public_test.go diff --git a/contracts/agents-api/openapi.yaml b/contracts/agents-api/openapi.yaml index 9d9590d6a..336c8f545 100644 --- a/contracts/agents-api/openapi.yaml +++ b/contracts/agents-api/openapi.yaml @@ -3811,11 +3811,13 @@ paths: - Sessions /agents/sessions/{session_id}: delete: - description: Removes the Session and its history from the public API. Active - work receives a cancellation request; confirmation does not guarantee native - execution has stopped. Internal records and native history are retained pending - separate physical cleanup. Missing/repeated deletion locally returns 404; - exact hosted errors and overlapping stream timing remain unverified. + description: Removes a durably idle or failed Session and its history from the + public API. A Session with a queued, in-progress or waiting Turn, required + actions or pending input returns 409 conflict_error and is left unchanged; + cancel it and wait until it is idle before deleting. Repeating the deletion + of the caller's own deleted Session returns the same confirmation; missing + and foreign Sessions return 404. Internal records and native history are retained + pending separate physical cleanup; overlapping stream timing remains unverified. parameters: - description: agents=v1 in: header @@ -3846,6 +3848,10 @@ paths: description: Not Found schema: $ref: '#/definitions/v1.ErrorResponse' + "409": + description: Conflict + schema: + $ref: '#/definitions/v1.ErrorResponse' "413": description: Request Entity Too Large schema: diff --git a/services/agents-api/README.md b/services/agents-api/README.md index 72c7def80..869a935f8 100644 --- a/services/agents-api/README.md +++ b/services/agents-api/README.md @@ -236,15 +236,19 @@ are 20 and descending order; exact hosted limits/error semantics remain unverifi Session updates require the metadata field; null/empty clears it and an object replaces supplied pairs. An empty update body rejects before resource lookup. -Delete with `client.beta.agents.sessions.delete(session.id)`. Confirmation means -public removal: Session/history reads and new input become unavailable. Active -work receives a cancellation request; existing streams close on observing removal. -Already claimed work may still complete. Creation keys stay reserved; deletion -never affects other Sessions, saved Agents or their shared device. Internal records -are retained for execution settlement. Managed Docker deletion separately revokes -authority and reclaims owned compute/workspace/history; caller-managed compute -is not reclaimed by this service. Local repeated deletion returns 404 and creation-key reuse returns -409; exact hosted errors and overlapping stream timing are unverified. +Delete with `client.beta.agents.sessions.delete(session.id)`. Only a durably idle +or failed Session without required actions or pending input can be deleted; any +other Session returns 409 `conflict_error` and is left unchanged. Cancel its work +with an `agent.session.input.cancel` event, wait until it is idle, then delete it. +Confirmation means public removal: Session/history reads and new input become +unavailable, and existing streams close on observing removal. Creation keys stay +reserved; deletion never affects other Sessions, saved Agents or their shared +device. Internal records are retained for execution settlement. Managed Docker +deletion separately revokes authority and reclaims owned compute/workspace/history; +caller-managed compute is not reclaimed by this service. Repeating the deletion of +your own deleted Session returns the same confirmation, missing and foreign +Sessions return 404, and creation-key reuse returns 409; overlapping stream timing +is unverified. Codex command Items support live `agent.output.command_execution_output.delta` events when emitted by the connected daemon. Queries retain accumulated drafts and diff --git a/services/agents-api/internal/api/errors.go b/services/agents-api/internal/api/errors.go index c2c8fd08e..7bc00c0ce 100644 --- a/services/agents-api/internal/api/errors.go +++ b/services/agents-api/internal/api/errors.go @@ -25,7 +25,7 @@ func writeError(w http.ResponseWriter, status int, code, message string, param . kind = "server_error" } else if status == http.StatusUnauthorized { kind = "authentication_error" - } else if code == "not_found_error" || code == "invalid_beta" { + } else if code == "not_found_error" || code == "invalid_beta" || code == "conflict_error" { kind = code } var errorCode *string @@ -98,6 +98,9 @@ func writeStoreError(w http.ResponseWriter, r *http.Request, err error, notFound code = "" } writeError(w, http.StatusNotFound, code, "Resource not found.", notFoundParam...) + case errors.Is(err, store.ErrSessionNotIdle): + // Observed official status, type, code, null param and message. + writeError(w, http.StatusConflict, "conflict_error", "session must be durably idle or failed without required actions before deletion") case errors.Is(err, store.ErrTurnConflict): writeError(w, http.StatusConflict, "turn_conflict", "The Turn cannot accept this input in its current state.") case errors.Is(err, store.ErrIdempotencyConflict): diff --git a/services/agents-api/internal/api/errors_test.go b/services/agents-api/internal/api/errors_test.go index 61f006a57..6023325d3 100644 --- a/services/agents-api/internal/api/errors_test.go +++ b/services/agents-api/internal/api/errors_test.go @@ -66,3 +66,14 @@ func TestMissingBetaErrorAfterAuthentication(t *testing.T) { } } } + +// Session deletion conflicts use the observed official 409 fields. +func TestSessionDeletionConflictError(t *testing.T) { + response := httptest.NewRecorder() + request := httptest.NewRequest(http.MethodDelete, "/v1/agents/sessions/session", nil) + writeStoreError(response, request, fmt.Errorf("delete: %w", store.ErrSessionNotIdle)) + want := `{"error":{"message":"session must be durably idle or failed without required actions before deletion","type":"conflict_error","code":"conflict_error","param":null}}` + "\n" + if response.Code != http.StatusConflict || response.Body.String() != want { + t.Fatalf("response = %d %s", response.Code, response.Body) + } +} diff --git a/services/agents-api/internal/api/session_deletion.go b/services/agents-api/internal/api/session_deletion.go index 1ef76af01..f896b2cb3 100644 --- a/services/agents-api/internal/api/session_deletion.go +++ b/services/agents-api/internal/api/session_deletion.go @@ -11,14 +11,14 @@ import ( ) // @Summary Delete an execution Session -// @Description Removes the Session and its history from the public API. Active work receives a cancellation request; confirmation does not guarantee native execution has stopped. Internal records and native history are retained pending separate physical cleanup. Missing/repeated deletion locally returns 404; exact hosted errors and overlapping stream timing remain unverified. +// @Description Removes a durably idle or failed Session and its history from the public API. A Session with a queued, in-progress or waiting Turn, required actions or pending input returns 409 conflict_error and is left unchanged; cancel it and wait until it is idle before deleting. Repeating the deletion of the caller's own deleted Session returns the same confirmation; missing and foreign Sessions return 404. Internal records and native history are retained pending separate physical cleanup; overlapping stream timing remains unverified. // @Tags Sessions // @Produce json // @Security BearerAuth // @Param OpenAI-Beta header string true "agents=v1" // @Param session_id path string true "Session ID" // @Success 200 {object} v1.SessionDeleted -// @Failure 400,401,404,413,500 {object} v1.ErrorResponse +// @Failure 400,401,404,409,413,500 {object} v1.ErrorResponse // @Router /agents/sessions/{session_id} [delete] func (h *Handler) deleteSession(w http.ResponseWriter, r *http.Request) { body, ok := readJSONBody(w, r) diff --git a/services/agents-api/internal/store/environment_admission_test.go b/services/agents-api/internal/store/environment_admission_test.go index 70ae826e2..f1ea665fe 100644 --- a/services/agents-api/internal/store/environment_admission_test.go +++ b/services/agents-api/internal/store/environment_admission_test.go @@ -187,7 +187,10 @@ func TestEnvironmentAdmissionSettlementDoesNotCreateTurn(t *testing.T) { } case "deleted": expected = store.ErrNotFound - if err := h.s.DeleteSession(t.Context(), h.tenant, h.session.ID); err != nil { + if err := h.s.DeleteSession(t.Context(), h.tenant, h.session.ID); !errors.Is(err, store.ErrSessionNotIdle) { + t.Fatal("pending input deleted", err) + } + if err := h.s.CommitLegacyDeletion(t.Context(), h.tenant, h.session.ID); err != nil { t.Fatal(err) } case "disconnected": diff --git a/services/agents-api/internal/store/environment_claim_worker_test.go b/services/agents-api/internal/store/environment_claim_worker_test.go index bc45f7902..641550062 100644 --- a/services/agents-api/internal/store/environment_claim_worker_test.go +++ b/services/agents-api/internal/store/environment_claim_worker_test.go @@ -27,7 +27,10 @@ func TestWorkerReconcilesEnvironmentPromotionBeforeStart(t *testing.T) { } turnID := got.Receipts[0].TurnID if deleted { - if err := s.DeleteSession(t.Context(), tenant, pending.SessionID); err != nil { + if err := s.DeleteSession(t.Context(), tenant, pending.SessionID); !errors.Is(err, store.ErrSessionNotIdle) { + t.Fatal("claimed Session deleted", err) + } + if err := s.CommitLegacyDeletion(t.Context(), tenant, pending.SessionID); err != nil { t.Fatal(err) } } diff --git a/services/agents-api/internal/store/environment_initial_input_test.go b/services/agents-api/internal/store/environment_initial_input_test.go index 041d7b68b..0d8f7f815 100644 --- a/services/agents-api/internal/store/environment_initial_input_test.go +++ b/services/agents-api/internal/store/environment_initial_input_test.go @@ -262,6 +262,13 @@ func TestEnvironmentInitialInputExpiryHasNoTurnAndCannotReplay(t *testing.T) { if err != nil || len(after) < len(events) || !reflect.DeepEqual(events, after[:len(events)]) { t.Fatal("later work changed historical failure", err) } + // The later input is still pending, so deletion waits for it to settle. + if err := reopened.DeleteSession(t.Context(), tenant, session.ID); !errors.Is(err, ErrSessionNotIdle) { + t.Fatal("pending later input deleted", err) + } + if _, err := reopened.CancelEnvironmentInput(t.Context(), tenant, session.ID, later.ID); err != nil { + t.Fatal(err) + } if err := reopened.DeleteSession(t.Context(), tenant, session.ID); err != nil { t.Fatal(err) } diff --git a/services/agents-api/internal/store/environment_input_activity_test.go b/services/agents-api/internal/store/environment_input_activity_test.go index c0748dfc9..df9e42063 100644 --- a/services/agents-api/internal/store/environment_input_activity_test.go +++ b/services/agents-api/internal/store/environment_input_activity_test.go @@ -238,7 +238,10 @@ func TestEnvironmentInputActivityRecoversWaitingActionAndHidesDeletion(t *testin t.Fatal("retired generation changed activity", after, cursor, err) } environmentInputHistory(t, pool, session.ID, 0, 0) - if err := s.DeleteSession(t.Context(), tenant, session.ID); err != nil { + if err := s.DeleteSession(t.Context(), tenant, session.ID); !errors.Is(err, ErrSessionNotIdle) { + t.Fatal("waiting input deleted", err) + } + if err := s.commitLegacyDeletion(t.Context(), tenant, session.ID); err != nil { t.Fatal(err) } if _, err := s.GetSession(t.Context(), tenant, session.ID); !errors.Is(err, ErrNotFound) { diff --git a/services/agents-api/internal/store/environment_input_settlement_test.go b/services/agents-api/internal/store/environment_input_settlement_test.go index b6fca37a4..d163447e2 100644 --- a/services/agents-api/internal/store/environment_input_settlement_test.go +++ b/services/agents-api/internal/store/environment_input_settlement_test.go @@ -225,7 +225,13 @@ func TestEnvironmentInputDeletionSettlesPendingAndFencesPromotion(t *testing.T) done <- err }() } - if err := s.DeleteSession(ctx, tenant, session.ID); err != nil { + if !concurrent { + if err := s.DeleteSession(ctx, tenant, session.ID); !errors.Is(err, ErrSessionNotIdle) { + t.Fatal("pending input deleted", err) + } + } + // A marker from an earlier release still settles and fences the input. + if err := s.commitLegacyDeletion(ctx, tenant, session.ID); err != nil { t.Fatal(err) } if concurrent { diff --git a/services/agents-api/internal/store/environment_work_test.go b/services/agents-api/internal/store/environment_work_test.go index 57e41aa83..3df60457d 100644 --- a/services/agents-api/internal/store/environment_work_test.go +++ b/services/agents-api/internal/store/environment_work_test.go @@ -1,6 +1,7 @@ package store_test import ( + "errors" "testing" "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/store" @@ -25,7 +26,10 @@ func TestEnvironmentInputWorkFiltersAndPagesDevices(t *testing.T) { t.Fatal(err) } case "deleted": - if err := h.s.DeleteSession(t.Context(), h.tenant, pending.SessionID); err != nil { + if err := h.s.DeleteSession(t.Context(), h.tenant, pending.SessionID); !errors.Is(err, store.ErrSessionNotIdle) { + t.Fatal("pending input deleted", err) + } + if err := h.s.CommitLegacyDeletion(t.Context(), h.tenant, pending.SessionID); err != nil { t.Fatal(err) } case "unbound": diff --git a/services/agents-api/internal/store/export_test.go b/services/agents-api/internal/store/export_test.go index a592b4b89..97b98fcea 100644 --- a/services/agents-api/internal/store/export_test.go +++ b/services/agents-api/internal/store/export_test.go @@ -1,6 +1,8 @@ package store import ( + "context" + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/identity" "github.com/jackc/pgx/v5/pgxpool" "testing" @@ -25,3 +27,9 @@ func FixtureExecutorPrincipal(t *testing.T, s *Store, tenant string) identity.Pr // SkillArchive builds a minimal valid Skill archive for public HTTP fixtures. func SkillArchive(t *testing.T, marker string) []byte { return skillArchive(t, marker) } + +// CommitLegacyDeletion commits a deletion marker the way releases before the +// idle-only deletion rule did, for Sessions that public deletion now rejects. +func (s *Store) CommitLegacyDeletion(ctx context.Context, tenantID, sessionID string) error { + return s.commitLegacyDeletion(ctx, tenantID, sessionID) +} diff --git a/services/agents-api/internal/store/prepared_dispatch_failure_test.go b/services/agents-api/internal/store/prepared_dispatch_failure_test.go index 42b167474..47d579096 100644 --- a/services/agents-api/internal/store/prepared_dispatch_failure_test.go +++ b/services/agents-api/internal/store/prepared_dispatch_failure_test.go @@ -30,7 +30,10 @@ func TestPreparedDispatchSettlesOnlyReadyInput(t *testing.T) { t.Fatal(err) } case "delete": - if err := h.s.DeleteSession(t.Context(), h.tenant, h.session.ID); err != nil { + if err := h.s.DeleteSession(t.Context(), h.tenant, h.session.ID); !errors.Is(err, store.ErrSessionNotIdle) { + t.Fatal("pending input deleted", err) + } + if err := h.s.CommitLegacyDeletion(t.Context(), h.tenant, h.session.ID); err != nil { t.Fatal(err) } case "prepare-failure": diff --git a/services/agents-api/internal/store/session_artifacts_test.go b/services/agents-api/internal/store/session_artifacts_test.go index b9233a638..a22a6675a 100644 --- a/services/agents-api/internal/store/session_artifacts_test.go +++ b/services/agents-api/internal/store/session_artifacts_test.go @@ -253,7 +253,11 @@ func TestSessionArtifactTransferDoesNotBlockDeletionOrCancellation(t *testing.T) defer cancel() want := ErrNotFound if operation == "delete" { - if err := s.DeleteSession(ctx, tenant, session); err != nil { + // The idle-only decision itself is not blocked by the transfer. + if err := s.DeleteSession(ctx, tenant, session); !errors.Is(err, ErrSessionNotIdle) { + t.Fatalf("transfer blocked or bypassed the deletion rule: %v", err) + } + if err := s.commitLegacyDeletion(ctx, tenant, session); err != nil { t.Fatalf("transfer blocked deletion: %v", err) } } else { diff --git a/services/agents-api/internal/store/session_deletion.go b/services/agents-api/internal/store/session_deletion.go index d1c64bfe3..9eee3c9f8 100644 --- a/services/agents-api/internal/store/session_deletion.go +++ b/services/agents-api/internal/store/session_deletion.go @@ -9,22 +9,62 @@ import ( "github.com/jackc/pgx/v5/pgtype" ) -// DeleteSession removes public access while retaining state needed to settle execution. +// ErrSessionNotIdle rejects deletion of a Session that still has work or input +// pending. Callers cancel first and delete after the Session settles. +var ErrSessionNotIdle = errors.New("session must be durably idle or failed without required actions before deletion") + +// errSessionAlreadyDeleted rolls back a repeated deletion without any write. +var errSessionAlreadyDeleted = errors.New("session already deleted") + +// DeleteSession removes public access to a durably idle or failed Session while +// retaining state needed to settle execution. The decision is taken under the +// Session lock that also orders Turn and input admission, so a concurrent +// admission either commits first and is rejected here, or observes the deletion. +// The owner's repeated deletion succeeds without another write; foreign and +// missing Sessions remain not found. func (s *Store) DeleteSession(ctx context.Context, tenantID, sessionID string) error { - return s.withPublicSession(ctx, tenantID, sessionID, func(ctx context.Context, q *sqlc.Queries, session pgtype.UUID) error { - if err := cancelSessionWork(ctx, q, session); err != nil { + err := s.withLockedSession(ctx, tenantID, sessionID, true, func(ctx context.Context, q *sqlc.Queries, session sqlc.LockSessionRow) error { + if session.DeletedAt.Valid { + return errSessionAlreadyDeleted + } + if err := requireSessionSettled(ctx, q, session.ID); err != nil { return err } - if err := q.DeleteSessionArtifacts(ctx, session); err != nil { + if err := q.DeleteSessionArtifacts(ctx, session.ID); err != nil { return err } - if err := q.ReleaseUnallocatedRuntimePlacement(ctx, session); err != nil { + if err := q.ReleaseUnallocatedRuntimePlacement(ctx, session.ID); err != nil { return err } - return q.MarkSessionDeleted(ctx, session) + return q.MarkSessionDeleted(ctx, session.ID) }) + if errors.Is(err, errSessionAlreadyDeleted) { + return nil + } + return err +} + +// requireSessionSettled rejects a queued, in-progress or waiting Turn, which +// includes pending required actions and function results, and a pending input +// reservation: queued later input, self-hosted input awaiting a connection, or +// hosted initial input while provisioning. Terminal idle and failed Sessions pass. +func requireSessionSettled(ctx context.Context, q *sqlc.Queries, session pgtype.UUID) error { + if _, err := q.GetActiveTurn(ctx, session); err == nil { + return ErrSessionNotIdle + } else if !errors.Is(err, pgx.ErrNoRows) { + return err + } + _, pending, err := environmentInputState(ctx, q, session) + if err != nil { + return err + } + if pending { + return ErrSessionNotIdle + } + return nil } +// cancelSessionWork requests cancellation of active work for Runtime cleanup. func cancelSessionWork(ctx context.Context, q *sqlc.Queries, session pgtype.UUID) error { turn, err := q.GetActiveTurn(ctx, session) if err != nil && !errors.Is(err, pgx.ErrNoRows) { diff --git a/services/agents-api/internal/store/session_deletion_execution_test.go b/services/agents-api/internal/store/session_deletion_execution_test.go index 3adb71db2..11ef5efa6 100644 --- a/services/agents-api/internal/store/session_deletion_execution_test.go +++ b/services/agents-api/internal/store/session_deletion_execution_test.go @@ -34,7 +34,11 @@ func TestDeletedSessionWaitingTurnSettlesWithoutStoppingWorker(t *testing.T) { h.read(proto.TypePromptRequest) h.write(input.TurnID, proto.TypeFunctionCall, proto.FunctionCallPayload{CallID: "pending", Name: "lookup_ticket", Arguments: json.RawMessage(`{}`)}) state := functionState(t, h, 1) - if err := h.s.DeleteSession(ctx, h.tenant, h.session.ID); err != nil { + if err := h.s.DeleteSession(ctx, h.tenant, h.session.ID); !errors.Is(err, store.ErrSessionNotIdle) { + t.Fatal("waiting Session deleted", err) + } + // A marker committed by an earlier release still cancels and settles work. + if err := h.s.CommitLegacyDeletion(ctx, h.tenant, h.session.ID); err != nil { t.Fatal(err) } var request proto.PromptCancelPayload @@ -71,7 +75,10 @@ func TestDeletedSessionRestartStillReconcilesHiddenClaim(t *testing.T) { if _, err := h.s.TransitionTurn(ctx, h.tenant, h.session.ID, input.TurnID, store.TurnTransition{ExpectedStatus: store.TurnQueued, Status: store.TurnInProgress}); err != nil { t.Fatal(err) } - if err := h.s.DeleteSession(ctx, h.tenant, h.session.ID); err != nil { + if err := h.s.DeleteSession(ctx, h.tenant, h.session.ID); !errors.Is(err, store.ErrSessionNotIdle) { + t.Fatal("running Session deleted", err) + } + if err := h.s.CommitLegacyDeletion(ctx, h.tenant, h.session.ID); err != nil { t.Fatal(err) } worker, err := execution.StartWorker(ctx, h.d) @@ -91,3 +98,74 @@ func TestDeletedSessionRestartStillReconcilesHiddenClaim(t *testing.T) { t.Fatal(err) } } + +// A caller deletes running work by cancelling first, waiting for the Session to +// settle and then deleting. The rejected deletion leaves the waiting Turn intact. +func TestWaitingSessionCancelsThenDeletesThroughWorker(t *testing.T) { + h := newFunctionHarness(t) + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + worker, err := execution.StartWorker(ctx, h.d) + if err != nil { + t.Fatal(err) + } + done := make(chan error, 1) + go func() { done <- worker.Run(ctx) }() + defer func() { + cancel() + select { + case <-done: + case <-time.After(10 * time.Second): + t.Error("worker did not stop") + } + }() + input := h.message("start", "Run") + h.read(proto.TypePromptRequest) + h.write(input.TurnID, proto.TypeFunctionCall, proto.FunctionCallPayload{CallID: "pending", Name: "lookup_ticket", Arguments: json.RawMessage(`{}`)}) + state := functionState(t, h, 1) + if err := h.s.DeleteSession(ctx, h.tenant, h.session.ID); !errors.Is(err, store.ErrSessionNotIdle) { + t.Fatal("waiting Session deleted", err) + } + if again := functionState(t, h, 1); again.LastTurn == nil || again.LastTurn.Status != store.TurnWaiting || !again.LastTurn.CancelRequestedAt.IsZero() { + t.Fatal("rejected deletion changed required actions", again) + } + if _, err := h.s.RequestCancel(ctx, h.tenant, h.session.ID, "cancel-before-delete"); err != nil { + t.Fatal(err) + } + // The explicit cancellation, not the rejected deletion, reaches the daemon. + var request proto.PromptCancelPayload + if err := h.read(proto.TypePromptCancel).DecodePayload(&request); err != nil { + t.Fatal(err) + } + if err := h.s.DeleteSession(ctx, h.tenant, h.session.ID); !errors.Is(err, store.ErrSessionNotIdle) { + t.Fatal("Session deleted before cancellation settled", err) + } + h.write(input.TurnID, proto.TypeInteractionDecisionAck, proto.InteractionDecisionAckPayload{DeliveryID: request.DeliveryID, Applied: true, Outcome: &proto.DonePayload{Metadata: map[string]any{proto.DoneMetaAgentSessionID: "cancelled-native"}}}) + waitTurn(t, h, input.TurnID, store.TurnCancelled) + deadline := time.Now().Add(10 * time.Second) + for { + err := h.s.DeleteSession(ctx, h.tenant, h.session.ID) + if err == nil { + break + } + if !errors.Is(err, store.ErrSessionNotIdle) || time.Now().After(deadline) { + t.Fatal("settled Session not deleted", err) + } + time.Sleep(20 * time.Millisecond) + } + if err := h.s.DeleteSession(ctx, h.tenant, h.session.ID); err != nil { + t.Fatal("repeated deletion", err) + } + raw, _ := json.Marshal(store.FunctionResultInput{TurnID: input.TurnID, CallID: state.RequiredActions[0].CallID, Result: json.RawMessage(`{"success":true,"output":"late"}`)}) + if _, err := worker.SubmitInputs(ctx, h.tenant, h.session.ID, "late", []store.Input{{Kind: "tool_result", Payload: raw}}); !errors.Is(err, store.ErrNotFound) { + t.Fatal(err) + } + if _, err := h.s.GetSession(ctx, h.tenant, h.session.ID); !errors.Is(err, store.ErrNotFound) { + t.Fatal(err) + } + h.session = publicSession(t, h, "unrelated") + next := h.message("next", "Unrelated work") + h.read(proto.TypePromptRequest) + h.write(next.TurnID, proto.TypeDone, proto.DonePayload{Content: "unaffected"}) + waitTurn(t, h, next.TurnID, store.TurnCompleted) +} diff --git a/services/agents-api/internal/store/session_deletion_lifecycle_public_test.go b/services/agents-api/internal/store/session_deletion_lifecycle_public_test.go new file mode 100644 index 000000000..f64cff5d3 --- /dev/null +++ b/services/agents-api/internal/store/session_deletion_lifecycle_public_test.go @@ -0,0 +1,230 @@ +package store_test + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "reflect" + "testing" + "time" + + "github.com/MiniMax-AI-Dev/parsar/internal/agentdaemon/device" + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/api" + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/store" + "github.com/google/uuid" +) + +const deletionAgent = `"agent":{"id":"agent_deletion","model":"fixture","tools":[],"multi_agent":{"enabled":false,"max_concurrent_subagents":null},"reasoning":{},"service_tier":"auto","text":{"format":{"type":"text"},"verbosity":"medium"}}` + +// TestSessionDeletionLifecyclePostgres replays the official deletion lifecycle +// over HTTP and PostgreSQL: busy Sessions conflict without any database write, +// settled Sessions delete once and confirm again for their owner, and foreign, +// missing and malformed identifiers keep one not-found response. +func TestSessionDeletionLifecyclePostgres(t *testing.T) { + // An isolated database keeps the no-write digest independent of other tests. + s, pool := store.NewManagedTestStore(t) + ctx := t.Context() + tenant, owner, foreign := uuid.NewString(), uuid.NewString(), uuid.NewString() + auth, err := api.NewAuthenticator([]api.APIKey{ + {OrganizationID: "test-org", ProjectID: tenant, SubjectKind: "service_account", SubjectID: "deletion-owner", TokenSHA256: device.HashCredential(owner), TenantID: tenant}, + {OrganizationID: "test-org", ProjectID: uuid.NewString(), SubjectKind: "service_account", SubjectID: "deletion-foreign", TokenSHA256: device.HashCredential(foreign), TenantID: uuid.NewString()}, + }) + if err != nil { + t.Fatal(err) + } + h, err := api.NewHandler(s, auth, "codex", api.WithEnvironmentRemoteURL("https://executor.example")) + if err != nil { + t.Fatal(err) + } + server := httptest.NewServer(h) + defer server.Close() + client := pathIDClient{t: t, server: server} + lease, err := s.AcquireExecutionLease(ctx) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + closing, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + _ = lease.Close(closing) + }) + writer := lease.Store() + + create := func(environment string, initial bool) store.Session { + t.Helper() + input := store.CreateSessionInput{Creator: store.FixtureCreator(), Engine: "codex", IdempotencyKey: uuid.NewString(), + Configuration: json.RawMessage(`{` + deletionAgent + `,"environment":` + environment + `}`)} + if initial { + input.InitialInputs = []store.Input{{Kind: "message", Payload: json.RawMessage(`{"text":"reserved"}`)}} + } + session, err := s.CreateSession(ctx, tenant, input) + if err != nil { + t.Fatal(err) + } + return session + } + none := `{"type":"none"}` + selfHosted := `{"type":"self_hosted","workspace_directory":"/workspace","capability_directories":[]}` + hosted := `{"type":"openai_hosted","network":{"access":"disabled"}}` + turn := func(to ...string) string { + t.Helper() + session := create(none, false) + receipt, err := s.SubmitMessage(ctx, tenant, session.ID, "input", json.RawMessage(`{"text":"work"}`)) + if err != nil { + t.Fatal(err) + } + from := store.TurnQueued + for _, status := range to { + switch status { + case "cancel": + _, err = s.RequestCancel(ctx, tenant, session.ID, "cancel") + case "function": + err = s.RecordFunctionCall(ctx, tenant, session.ID, receipt.TurnID, store.FunctionCall{CallID: "pending", ExecutorCallID: "native-pending", Name: "lookup", Arguments: json.RawMessage(`{}`)}) + case store.TurnCompleted, store.TurnFailed: + _, err = s.CompleteExecution(ctx, tenant, session.ID, receipt.TurnID, status, nil, "", receipt.Sequence) + default: + _, err = s.TransitionTurn(ctx, tenant, session.ID, receipt.TurnID, store.TurnTransition{ExpectedStatus: from, Status: status}) + from = status + } + if err != nil { + t.Fatal(status, err) + } + } + return session.ID + } + reserve := func(session store.Session) store.EnvironmentInputReservation { + t.Helper() + reservation, err := s.ReserveEnvironmentInput(ctx, tenant, session.ID, "later", []store.Input{{Kind: "message", Payload: json.RawMessage(`{"text":"later"}`)}}) + if err != nil || reservation.State != store.EnvironmentInputPending { + t.Fatal(reservation, err) + } + return reservation + } + connect := func(session store.Session) { + t.Helper() + generation := uuid.NewString() + if err := writer.ReplaceEnvironmentConnection(ctx, tenant, session.Environment.ID, generation); err != nil { + t.Fatal(err) + } + if err := writer.ObserveEnvironmentConnection(ctx, tenant, session.Environment.ID, generation, 1, true); err != nil { + t.Fatal(err) + } + } + + // D3: every Session that is not durably idle or failed without required actions. + busy := map[string]string{ + "queued_turn": turn(), + "in_progress_turn": turn(store.TurnInProgress), + "cancelling_turn": turn(store.TurnInProgress, "cancel"), + "required_action": turn(store.TurnInProgress, "function"), + } + awaiting := create(selfHosted, true) + busy["self_hosted_awaiting_connection"] = awaiting.ID + queued := create(selfHosted, false) + connect(queued) + reserve(queued) + busy["self_hosted_queued_input"] = queued.ID + busy["hosted_provisioning_input"] = create(hosted, true).ID + // Pending input can project idle while it still waits for admission. + projected := map[string]string{ + "queued_turn": "in_progress", "in_progress_turn": "in_progress", "cancelling_turn": "in_progress", + "required_action": "requires_action", "self_hosted_awaiting_connection": "requires_action", + "self_hosted_queued_input": "idle", "hosted_provisioning_input": "idle", + } + + // D4: settled Sessions, including idle hosted provisioning without input. + settled := map[string]string{ + "none_idle": create(none, false).ID, + "completed_turn": turn(store.TurnInProgress, store.TurnCompleted), + "failed_turn": turn(store.TurnInProgress, store.TurnFailed), + "cancelled_turn": turn("cancel"), + "self_hosted_idle": create(selfHosted, false).ID, + "hosted_provisioning_idle": create(hosted, false).ID, + "self_hosted_input_expired": "", + "later_input_cancelled": "", + } + expired := create(selfHosted, true) + if _, err := pool.Exec(ctx, "UPDATE environment_input_reservations SET deadline=clock_timestamp()-interval '1 second' WHERE session_id=$1", expired.ID); err != nil { + t.Fatal(err) + } + if count, err := writer.ExpireEnvironmentInputs(ctx); err != nil || count != 1 { + t.Fatal("initial input did not expire", count, err) + } + settled["self_hosted_input_expired"] = expired.ID + withdrawn := create(selfHosted, false) + if _, err := s.CancelEnvironmentInput(ctx, tenant, withdrawn.ID, reserve(withdrawn).ID); err != nil { + t.Fatal(err) + } + settled["later_input_cancelled"] = withdrawn.ID + + sessionPath := func(id string) string { return "/v1/agents/sessions/" + id } + decode := func(raw string) map[string]any { + t.Helper() + var value map[string]any + if err := json.Unmarshal([]byte(raw), &value); err != nil { + t.Fatal(raw, err) + } + return value + } + missingStatus, missing := client.do(owner, http.MethodDelete, sessionPath(uuid.NewString()), "", nil) + if missingStatus != http.StatusNotFound { + t.Fatal(missingStatus, missing) + } + if !reflect.DeepEqual(decode(missing), map[string]any{"error": map[string]any{ + "type": "not_found_error", "code": "not_found_error", "message": "Resource not found.", "param": nil}}) { + t.Fatal("missing Session body", missing) + } + notFound := func(token, method, path string) { + t.Helper() + if status, raw := client.do(token, method, path, "", nil); status != http.StatusNotFound || raw != missing { + t.Fatalf("%s %s: %d %s", method, path, status, raw) + } + } + notFound(owner, http.MethodDelete, sessionPath("sess_malformed")) + conflict := map[string]any{"error": map[string]any{ + "type": "conflict_error", "code": "conflict_error", "param": nil, + "message": "session must be durably idle or failed without required actions before deletion"}} + + for name, id := range busy { + t.Run("conflict/"+name, func(t *testing.T) { + readStatus, before := client.do(owner, http.MethodGet, sessionPath(id), "", nil) + if readStatus != http.StatusOK || decode(before)["status"] != projected[name] { + t.Fatal(readStatus, before) + } + digest := databaseDigest(t, pool) + notFound(foreign, http.MethodDelete, sessionPath(id)) + status, raw := client.do(owner, http.MethodDelete, sessionPath(id), "", nil) + if status != http.StatusConflict || !reflect.DeepEqual(decode(raw), conflict) { + t.Fatalf("busy Session deletion: %d %s", status, raw) + } + if after := databaseDigest(t, pool); !reflect.DeepEqual(after, digest) { + t.Fatal("rejected deletion changed the database") + } + if readStatus, after := client.do(owner, http.MethodGet, sessionPath(id), "", nil); readStatus != http.StatusOK || after != before { + t.Fatal("rejected deletion changed the Session", before, after) + } + }) + } + for name, id := range settled { + t.Run("deleted/"+name, func(t *testing.T) { + notFound(foreign, http.MethodDelete, sessionPath(id)) + status, first := client.do(owner, http.MethodDelete, sessionPath(id), "", nil) + if status != http.StatusOK || !reflect.DeepEqual(decode(first), map[string]any{"id": id, "object": "agent.session.deleted", "deleted": true}) { + t.Fatalf("settled Session deletion: %d %s", status, first) + } + digest := databaseDigest(t, pool) + for range 2 { + if status, again := client.do(owner, http.MethodDelete, sessionPath(id), "", nil); status != http.StatusOK || again != first { + t.Fatalf("repeated deletion: %d %s", status, again) + } + } + if after := databaseDigest(t, pool); !reflect.DeepEqual(after, digest) { + t.Fatal("repeated deletion changed the database") + } + notFound(owner, http.MethodGet, sessionPath(id)) + notFound(owner, http.MethodGet, sessionPath(id)+"/turns") + notFound(foreign, http.MethodDelete, sessionPath(id)) + }) + } +} diff --git a/services/agents-api/internal/store/session_deletion_test.go b/services/agents-api/internal/store/session_deletion_test.go index f85da918e..84ca0f338 100644 --- a/services/agents-api/internal/store/session_deletion_test.go +++ b/services/agents-api/internal/store/session_deletion_test.go @@ -4,16 +4,52 @@ import ( "context" "encoding/json" "errors" + "strings" + "sync" "testing" + "time" + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/db/sqlc" "github.com/google/uuid" + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgtype" + "github.com/jackc/pgx/v5/pgxpool" ) -func TestSessionDeletionPreservesExecutionAndRejectsAdmission(t *testing.T) { +// commitLegacyDeletion reproduces the deletion transaction of releases before +// the idle-only rule: it requested cancellation of pending work and committed +// the marker together. Upgraded databases can retain such markers, so hidden +// work must still settle; tests use this to reach that state. +func (s *Store) commitLegacyDeletion(ctx context.Context, tenantID, sessionID string) error { + return s.withPublicSession(ctx, tenantID, sessionID, func(ctx context.Context, q *sqlc.Queries, session pgtype.UUID) error { + if err := cancelSessionWork(ctx, q, session); err != nil { + return err + } + if err := q.DeleteSessionArtifacts(ctx, session); err != nil { + return err + } + if err := q.ReleaseUnallocatedRuntimePlacement(ctx, session); err != nil { + return err + } + return q.MarkSessionDeleted(ctx, session) + }) +} + +// sessionDeletedAt reads the internal marker, which public reads never expose. +func sessionDeletedAt(t *testing.T, pool *pgxpool.Pool, session string) pgtype.Timestamptz { + t.Helper() + var deleted pgtype.Timestamptz + if err := pool.QueryRow(t.Context(), "SELECT deleted_at FROM sessions WHERE id=$1", session).Scan(&deleted); err != nil { + t.Fatal(err) + } + return deleted +} + +func TestSessionDeletionWaitsForSettledTurnAndRejectsAdmission(t *testing.T) { s, pool := testStore(t) ctx := t.Context() tenant := uuid.NewString() - for _, status := range []string{TurnQueued, TurnInProgress, TurnCompleted} { + for _, status := range []string{TurnQueued, TurnInProgress, TurnCompleted, TurnFailed} { t.Run(status, func(t *testing.T) { input := CreateSessionInput{Creator: FixtureCreator(), Engine: "codex", IdempotencyKey: status} session, err := s.CreateSession(ctx, tenant, input) @@ -30,19 +66,64 @@ func TestSessionDeletionPreservesExecutionAndRejectsAdmission(t *testing.T) { t.Fatal(err) } } - if status == TurnCompleted { - _, err = s.CompleteExecution(ctx, tenant, session.ID, receipt.TurnID, TurnCompleted, nil, "", receipt.Sequence) - if err != nil { + if status == TurnCompleted || status == TurnFailed { + if _, err = s.CompleteExecution(ctx, tenant, session.ID, receipt.TurnID, status, nil, "", receipt.Sequence); err != nil { t.Fatal(err) } } if err := s.DeleteSession(ctx, uuid.NewString(), session.ID); !errors.Is(err, ErrNotFound) { t.Fatal(err) } + if status == TurnQueued || status == TurnInProgress { + // Deletion leaves active work untouched: no cancellation, marker or event. + cursor, err := s.SessionEventCursor(ctx, tenant, session.ID) + if err != nil { + t.Fatal(err) + } + if err := s.DeleteSession(ctx, tenant, session.ID); !errors.Is(err, ErrSessionNotIdle) { + t.Fatal("active Session deleted", err) + } + turn, err := s.GetTurn(ctx, tenant, session.ID, receipt.TurnID) + if err != nil || turn.Status != status || !turn.CancelRequestedAt.IsZero() { + t.Fatal("rejected deletion changed the Turn", turn, err) + } + if after, err := s.SessionEventCursor(ctx, tenant, session.ID); err != nil || after != cursor { + t.Fatal("rejected deletion recorded an event", after, cursor, err) + } + if sessionDeletedAt(t, pool, session.ID).Valid { + t.Fatal("rejected deletion committed a marker") + } + // Callers cancel first. A queued Turn cancels at once; a running + // Turn stays active until execution settles its cancellation. + if _, err := s.RequestCancel(ctx, tenant, session.ID, "cancel"); err != nil { + t.Fatal(err) + } + if status == TurnInProgress { + if err := s.DeleteSession(ctx, tenant, session.ID); !errors.Is(err, ErrSessionNotIdle) { + t.Fatal("cancelling Session deleted", err) + } + if _, err := s.CompleteExecution(ctx, tenant, session.ID, receipt.TurnID, TurnCancelled, nil, "", receipt.Sequence); err != nil { + t.Fatal(err) + } + } + } if err := s.DeleteSession(ctx, tenant, session.ID); err != nil { t.Fatal(err) } + marker := sessionDeletedAt(t, pool, session.ID) fresh := New(pool) + // The owner's repeated deletion confirms again without another write. + for _, repeat := range []*Store{s, fresh} { + if err := repeat.DeleteSession(ctx, tenant, session.ID); err != nil { + t.Fatal("repeated deletion", err) + } + if err := repeat.DeleteSession(ctx, uuid.NewString(), session.ID); !errors.Is(err, ErrNotFound) { + t.Fatal("foreign deleted Session", err) + } + } + if again := sessionDeletedAt(t, pool, session.ID); again != marker { + t.Fatal("repeated deletion rewrote the marker", marker, again) + } if _, err := fresh.GetSession(ctx, tenant, session.ID); !errors.Is(err, ErrNotFound) { t.Fatal(err) } @@ -55,6 +136,9 @@ func TestSessionDeletionPreservesExecutionAndRejectsAdmission(t *testing.T) { if _, err := fresh.SubmitMessage(ctx, tenant, session.ID, "input", json.RawMessage(`{"text":"retained"}`)); !errors.Is(err, ErrNotFound) { t.Fatal(err) } + if _, err := fresh.RequestCancel(ctx, tenant, session.ID, "late-cancel"); !errors.Is(err, ErrNotFound) { + t.Fatal(err) + } if _, err := fresh.ListItems(ctx, tenant, session.ID, "", 20, true); !errors.Is(err, ErrNotFound) { t.Fatal(err) } @@ -62,25 +146,18 @@ func TestSessionDeletionPreservesExecutionAndRejectsAdmission(t *testing.T) { if err != nil { t.Fatal(err) } - if status == TurnQueued { - if turn.Status != TurnCancelled { - t.Fatal(turn) - } - if _, err := fresh.TransitionTurn(ctx, tenant, session.ID, receipt.TurnID, TurnTransition{ExpectedStatus: TurnQueued, Status: TurnInProgress}); !errors.Is(err, ErrTurnConflict) { - t.Fatal(err) - } - } else if status == TurnInProgress { - if turn.CancelRequestedAt.IsZero() { - t.Fatal("missing internal cancellation") - } - if _, err := fresh.CompleteExecution(ctx, tenant, session.ID, receipt.TurnID, TurnCancelled, nil, "", receipt.Sequence); err != nil { - t.Fatal(err) - } - } else if turn.Status != TurnCompleted { + want := status + if status == TurnQueued || status == TurnInProgress { + want = TurnCancelled + } + if turn.Status != want { t.Fatal(turn) } + if _, err := fresh.TransitionTurn(ctx, tenant, session.ID, receipt.TurnID, TurnTransition{ExpectedStatus: TurnQueued, Status: TurnInProgress}); !errors.Is(err, ErrTurnConflict) { + t.Fatal(err) + } inputs, err := fresh.ListTurnInputs(ctx, tenant, session.ID, receipt.TurnID, 0, 20) - if err != nil || len(inputs) != 1 { + if err != nil || len(inputs) == 0 || inputs[0].Sequence != receipt.Sequence { t.Fatal(inputs, err) } if _, err := fresh.SessionEventCursor(ctx, tenant, session.ID); !errors.Is(err, ErrNotFound) { @@ -121,3 +198,167 @@ func TestSessionDeletionSerializesAdmissionBeforeRetryLookup(t *testing.T) { t.Fatal("retry admitted after deletion", err) } } + +// afterSessionLock runs once, inside the traced transaction, right after it +// acquires the Session row lock. +type afterSessionLock struct { + once sync.Once + run func() +} + +type sessionLockQuery struct{} + +func (a *afterSessionLock) TraceQueryStart(ctx context.Context, _ *pgx.Conn, data pgx.TraceQueryStartData) context.Context { + return context.WithValue(ctx, sessionLockQuery{}, strings.HasPrefix(data.SQL, "-- name: LockSession ")) +} + +func (a *afterSessionLock) TraceQueryEnd(ctx context.Context, _ *pgx.Conn, data pgx.TraceQueryEndData) { + if locked, _ := ctx.Value(sessionLockQuery{}).(bool); locked && data.Err == nil { + a.once.Do(a.run) + } +} + +// awaitSessionLockWaiter waits until another connection blocks on a Session lock. +func awaitSessionLockWaiter(t *testing.T, pool *pgxpool.Pool) { + t.Helper() + deadline := time.Now().Add(10 * time.Second) + for { + var waiting int + err := pool.QueryRow(context.Background(), `SELECT count(*) FROM pg_stat_activity + WHERE datname = current_database() AND wait_event_type = 'Lock' AND query LIKE '-- name: LockSession %'`).Scan(&waiting) + if err != nil { + t.Error(err) + return + } + if waiting > 0 { + return + } + if time.Now().After(deadline) { + t.Error("competing operation never waited for the Session lock") + return + } + time.Sleep(10 * time.Millisecond) + } +} + +// TestSessionDeletionRacesAdmissionUnderSessionLock runs the production deletion +// and admission paths against one real PostgreSQL row lock. Whichever commits +// first decides: admitted work makes deletion conflict without mutation, and a +// committed deletion makes admission not found. Both never succeed. +func TestSessionDeletionRacesAdmissionUnderSessionLock(t *testing.T) { + type admission struct { + name string + setup func(t *testing.T, s *Store) (string, string) + admit func(ctx context.Context, s *Store, tenant, session string) error + } + admissions := []admission{ + {"turn", func(t *testing.T, s *Store) (string, string) { + tenant, session := newTurnSession(t, s) + return tenant, session.ID + }, func(ctx context.Context, s *Store, tenant, session string) error { + _, err := s.SubmitMessage(ctx, tenant, session, "racing", messagePayload) + return err + }}, + {"environment_input", func(t *testing.T, s *Store) (string, string) { + tenant, session := environmentInputSession(t, s) + return tenant, session.ID + }, func(ctx context.Context, s *Store, tenant, session string) error { + _, err := s.ReserveEnvironmentInput(ctx, tenant, session, "racing", []Input{messageInput("racing")}) + return err + }}, + } + // An isolated database keeps lock-wait observation independent of other tests. + plain, pool := newManagedTestStore(t) + traced := func(t *testing.T, run func()) *Store { + cfg := pool.Config().Copy() + cfg.ConnConfig.Tracer = &afterSessionLock{run: run} + instrumented, err := pgxpool.NewWithConfig(t.Context(), cfg) + if err != nil { + t.Fatal(err) + } + t.Cleanup(instrumented.Close) + return New(instrumented) + } + for _, kind := range admissions { + t.Run(kind.name+"/admission-first", func(t *testing.T) { + tenant, session := kind.setup(t, plain) + deleted := make(chan error, 1) + s := traced(t, func() { + go func() { deleted <- plain.DeleteSession(context.Background(), tenant, session) }() + awaitSessionLockWaiter(t, pool) + }) + if err := kind.admit(t.Context(), s, tenant, session); err != nil { + t.Fatal(err) + } + if err := <-deleted; !errors.Is(err, ErrSessionNotIdle) { + t.Fatal("deletion ignored committed admission", err) + } + if sessionDeletedAt(t, pool, session).Valid { + t.Fatal("rejected deletion committed a marker") + } + current, err := plain.GetSession(t.Context(), tenant, session) + if err != nil { + t.Fatal(err) + } + if kind.name == "turn" && (current.LastTurn == nil || current.LastTurn.Status != TurnQueued || !current.LastTurn.CancelRequestedAt.IsZero()) { + t.Fatal("rejected deletion changed admitted work", current.LastTurn) + } + if kind.name == "environment_input" && !current.PendingInput { + t.Fatal("rejected deletion settled pending input", current) + } + }) + t.Run(kind.name+"/deletion-first", func(t *testing.T) { + tenant, session := kind.setup(t, plain) + admitted := make(chan error, 1) + s := traced(t, func() { + go func() { admitted <- kind.admit(context.Background(), plain, tenant, session) }() + awaitSessionLockWaiter(t, pool) + }) + if err := s.DeleteSession(t.Context(), tenant, session); err != nil { + t.Fatal(err) + } + if err := <-admitted; !errors.Is(err, ErrNotFound) { + t.Fatal("admission after deletion", err) + } + var turns, reservations int + if err := pool.QueryRow(t.Context(), `SELECT (SELECT count(*) FROM turns WHERE session_id=$1), + (SELECT count(*) FROM environment_input_reservations WHERE session_id=$1)`, session).Scan(&turns, &reservations); err != nil { + t.Fatal(err) + } + if turns != 0 || reservations != 0 { + t.Fatal("deleted Session admitted work", turns, reservations) + } + }) + t.Run(kind.name+"/concurrent", func(t *testing.T) { + for range 8 { + tenant, session := kind.setup(t, plain) + other := New(pool) + start := make(chan struct{}) + results := make(chan error, 2) + go func() { <-start; results <- plain.DeleteSession(context.Background(), tenant, session) }() + go func() { <-start; results <- kind.admit(context.Background(), other, tenant, session) }() + close(start) + first, second := <-results, <-results + deleted := sessionDeletedAt(t, pool, session).Valid + var active int + if err := pool.QueryRow(t.Context(), `SELECT (SELECT count(*) FROM turns WHERE session_id=$1) + + (SELECT count(*) FROM environment_input_reservations WHERE session_id=$1 AND state='pending')`, session).Scan(&active); err != nil { + t.Fatal(err) + } + // Exactly one side succeeds; the other reports the committed state. + failures := 0 + for _, err := range []error{first, second} { + if err != nil { + failures++ + if !errors.Is(err, ErrSessionNotIdle) && !errors.Is(err, ErrNotFound) { + t.Fatal(err) + } + } + } + if failures != 1 || deleted == (active > 0) { + t.Fatal("deletion and admission both decided", first, second, deleted, active) + } + } + }) + } +} diff --git a/services/agents-api/internal/store/session_transaction.go b/services/agents-api/internal/store/session_transaction.go index 54f742317..5ccce0b0b 100644 --- a/services/agents-api/internal/store/session_transaction.go +++ b/services/agents-api/internal/store/session_transaction.go @@ -21,6 +21,17 @@ func (s *Store) withPublicSession(ctx context.Context, tenantID, sessionID strin } func (s *Store) withSessionState(ctx context.Context, tenantID, sessionID string, public bool, apply func(context.Context, *sqlc.Queries, pgtype.UUID) error) error { + return s.withLockedSession(ctx, tenantID, sessionID, public, func(ctx context.Context, q *sqlc.Queries, session sqlc.LockSessionRow) error { + if public && session.DeletedAt.Valid { + return ErrNotFound + } + return apply(ctx, q, session.ID) + }) +} + +// withLockedSession locks the tenant-owned Session row, including a publicly +// deleted one, and commits only when apply succeeds. +func (s *Store) withLockedSession(ctx context.Context, tenantID, sessionID string, public bool, apply func(context.Context, *sqlc.Queries, sqlc.LockSessionRow) error) error { tenant, err := parseID(tenantID) if err != nil { return err @@ -49,10 +60,7 @@ func (s *Store) withSessionState(ctx context.Context, tenantID, sessionID string } else if err != nil { return err } - if public && session.DeletedAt.Valid { - return ErrNotFound - } - if err := apply(ctx, q, id); err != nil { + if err := apply(ctx, q, session); err != nil { return err } return q.PruneSessionEvents(ctx, id) diff --git a/services/agents-api/internal/store/subagent_identities_test.go b/services/agents-api/internal/store/subagent_identities_test.go index b4b5c0232..d4a2870d6 100644 --- a/services/agents-api/internal/store/subagent_identities_test.go +++ b/services/agents-api/internal/store/subagent_identities_test.go @@ -130,7 +130,10 @@ func TestSubagentIdentityIsAtomicScopedAndImmutable(t *testing.T) { if err != nil || !reflect.DeepEqual(again, saved) { t.Fatal("continuation changed immutable first observation", again, err) } - if err = reopened.DeleteSession(ctx, tenant, session.ID); err != nil { + if err = reopened.DeleteSession(ctx, tenant, session.ID); !errors.Is(err, ErrSessionNotIdle) { + t.Fatal("running Session deleted", err) + } + if err = reopened.commitLegacyDeletion(ctx, tenant, session.ID); err != nil { t.Fatal(err) } if _, err = reopened.GetSubagentIdentity(ctx, tenant, session.ID, "child-a"); !errors.Is(err, ErrNotFound) { diff --git a/services/agents-api/internal/store/worker_input_race_test.go b/services/agents-api/internal/store/worker_input_race_test.go index b4ca8c2a3..8e87c1277 100644 --- a/services/agents-api/internal/store/worker_input_race_test.go +++ b/services/agents-api/internal/store/worker_input_race_test.go @@ -45,7 +45,9 @@ func TestWorkerInputReadSkipsConcurrentlyCancelledCandidate(t *testing.T) { mutated := make(chan error, 1) cfg.ConnConfig.Tracer = &beforeInputRead{run: func() { if deleted { - mutated <- h.s.DeleteSession(t.Context(), h.tenant, candidateSession) + // Public deletion now rejects the queued candidate; an + // earlier release's marker must still fence its execution. + mutated <- h.s.CommitLegacyDeletion(t.Context(), h.tenant, candidateSession) return } _, err := h.s.SubmitInputs(t.Context(), h.tenant, candidateSession, "cancel", []store.Input{{Kind: "cancel", Payload: json.RawMessage(`{}`)}}) diff --git a/services/agents-api/tests/official_session_delete.py b/services/agents-api/tests/official_session_delete.py index de3d9e945..600393d09 100644 --- a/services/agents-api/tests/official_session_delete.py +++ b/services/agents-api/tests/official_session_delete.py @@ -16,6 +16,10 @@ def rejected(status, action): raise AssertionError(f"expected local status {status}") +CONFLICT = {"error": {"type": "conflict_error", "code": "conflict_error", "param": None, + "message": "session must be durably idle or failed without required actions before deletion"}} + + def main(): base, token, foreign, restarted = sys.argv[1:] with httpx2.Client(trust_env=False, timeout=10) as http: @@ -53,6 +57,19 @@ def client(url, key): for body in ("null", "{}"): assert http.request("DELETE", endpoint, headers=headers, content=body).status_code == 400 assert sessions.retrieve(session.id).id == session.id + # The queued Turn is not durably idle: deletion conflicts and changes nothing. + before = sessions.retrieve(session.id) + assert before.status == "in_progress" and turn.status == "queued" + conflict = http.delete(endpoint, headers=headers) + assert conflict.status_code == 409 and conflict.json() == CONFLICT, conflict.text + rejected(409, lambda: sessions.delete(session.id)) + assert sessions.retrieve(session.id) == before + assert sessions.turns.retrieve(turn.id, session_id=session.id) == turn + rejected(404, lambda: other.delete(session.id)) + # Callers cancel first and delete once the Session is idle. + sessions.events.create(session.id, events=[{"type": "agent.session.input.cancel"}]) + assert sessions.retrieve(session.id).status == "idle" + assert sessions.turns.retrieve(turn.id, session_id=session.id).status == "cancelled" # Existing live streams close on public removal without a fabricated event. with http.stream("GET", endpoint + "/events", headers=headers) as stream: assert stream.status_code == 200 @@ -62,28 +79,33 @@ def client(url, key): assert raw.parse().to_dict() == expected assert not any(line.startswith(("event:", "data:")) for line in stream.iter_lines()) for reader in (sessions, recovered): + # The owner's repeated deletion returns the same confirmation. + repeated = reader.with_raw_response.delete(session.id) + assert repeated.status_code == 200 and repeated.http_response.json() == expected rejected(404, lambda: reader.retrieve(session.id)) rejected(404, lambda: reader.update(session.id, metadata={"no": "resurrection"})) rejected(404, lambda: list(reader.items.list(session.id))) rejected(404, lambda: list(reader.turns.list(session.id))) rejected(404, lambda: reader.turns.retrieve(turn.id, session_id=session.id)) - rejected(404, lambda: reader.delete(session.id)) for stream in (False, True): rejected(409, lambda: reader.create(**spec, stream=stream, extra_headers=key)) assert session.id not in {s.id for s in reader.list()} assert session.id not in {s.id for s in reader.list(agent_id=session.agent.id)} + rejected(404, lambda: other.delete(session.id)) assert http.get(endpoint + "/events", headers=headers).status_code == 404 assert http.post(endpoint + "/events", headers=headers, json={"events": [{"type": "agent.session.input.cancel"}]}).status_code == 404 fresh = sessions.create(**spec) assert fresh.id != session.id - sessions.delete(fresh.id) + rejected(409, lambda: sessions.delete(fresh.id)) + sessions.events.create(fresh.id, events=[{"type": "agent.session.input.cancel"}]) + assert sessions.delete(fresh.id).deleted assert sessions.retrieve(peer.id) == peer assert api.beta.agents.retrieve(agent.id) == agent assert other.retrieve(foreign_session.id) == foreign_session rejected(404, lambda: sessions.delete(foreign_session.id)) for missing in (str(uuid.uuid4()), "invalid", str(uuid.UUID(int=0))): rejected(404, lambda: sessions.delete(missing)) - print("Session deletion: fixed SDK/raw HTTP, public history/stream removal, tenant isolation, durable retry rejection and independent resources passed.") + print("Session deletion: fixed SDK/raw HTTP, busy-Session 409 without change, cancel-then-delete, idempotent owner repeat, public history/stream removal, tenant isolation, durable retry rejection and independent resources passed.") if __name__ == "__main__": From 22d22e0ab27ebb1c937aba7c67fabf4c493c02c7 Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 12:39:43 +0000 Subject: [PATCH 2/8] Offer cancel-then-delete when Core refuses a busy Session The client exposes isSessionDeletionConflict for the 409 conflict_error that Core returns for a Session with pending work or input. The Web delete dialog explains that conflict and replaces its action with an explicit Cancel work and delete: one cancellation, bounded reads until the Session is idle or failed without required actions, then one deletion. Rejected or uncertain cancellation, a timeout, a connection change or another 409 stop without retrying; other 409 responses keep the generic message. --- apps/web/src/App.tsx | 18 ++++ .../features/CoreCollectionStates.test.tsx | 1 + .../src/features/sessions/SessionsView.tsx | 3 + .../actions/SessionActionsDialog.test.tsx | 9 ++ .../sessions/actions/SessionActionsDialog.tsx | 30 ++++-- .../sessions/actions/session-actions.test.ts | 86 +++++++++++++++++- .../sessions/actions/session-actions.ts | 91 +++++++++++++++++++ docs/web/protocol-coverage.md | 11 ++- packages/agents-client/src/client.test.ts | 26 +++++- packages/agents-client/src/client.ts | 15 +++ packages/agents-client/src/index.ts | 2 +- 11 files changed, 280 insertions(+), 12 deletions(-) diff --git a/apps/web/src/App.tsx b/apps/web/src/App.tsx index 9a9a267df..ac7f85405 100644 --- a/apps/web/src/App.tsx +++ b/apps/web/src/App.tsx @@ -59,12 +59,14 @@ import { removeSession, reconcileUnknownSessionDelete, replaceSessionMetadata, + requestSessionCancelBeforeDelete, requestSessionDelete, requestSessionDetail, requestSessionUpdate, selectionAfterSessionDelete, SessionActionError, SessionMetadataConflictError, + waitForSessionIdle, } from "./features/sessions/actions/session-actions"; import { environmentObservationFromResource, @@ -114,6 +116,7 @@ import { } from "./lib/pending-function-result"; import { beginPendingSend, + createIdempotencyKey, failPendingSend, type FailedPendingSend, } from "./lib/pending-send"; @@ -2015,6 +2018,20 @@ export function App() { return removeSessionFromWorkspace(sessionId, "Session deleted from Agent Core."); }; + // Runs only after the user confirms Cancel work and delete for a Session that + // Core refused to delete: cancel once, read until idle, then delete once. + const cancelAndDeleteSessionFromCore = async (sessionId: string): Promise => { + const generation = coreGeneration; + const isCurrent = () => generation === connectionGenerationRef.current; + await requestSessionCancelBeforeDelete(core, sessionId, createIdempotencyKey()); + const settled = await waitForSessionIdle(core, sessionId, { isCurrent }); + if (settled === "stale" || !isCurrent()) return false; + if (settled === "missing") { + return removeSessionFromWorkspace(sessionId, "Session is absent from Agent Core after cancellation."); + } + return deleteSessionFromCore(sessionId); + }; + const sendMessage = async (text: string) => { const sessionId = selectedId; if (!sessionId) return; @@ -2358,6 +2375,7 @@ export function App() { onAgentFilterChange={changeSessionAgentFilter} onCreateSession={createSession} onDeleteSession={deleteSessionFromCore} + onCancelAndDeleteSession={cancelAndDeleteSessionFromCore} onFunctionResult={submitFunctionResult} onListEnvironmentFiles={listEnvironmentFiles} onCreateEnvironmentFile={createEnvironmentFile} diff --git a/apps/web/src/features/CoreCollectionStates.test.tsx b/apps/web/src/features/CoreCollectionStates.test.tsx index e6821aed7..1c1997c33 100644 --- a/apps/web/src/features/CoreCollectionStates.test.tsx +++ b/apps/web/src/features/CoreCollectionStates.test.tsx @@ -16,6 +16,7 @@ const sessionsCallbacks = { onCancel: async () => undefined, onCreateSession: async () => undefined, onDeleteSession: async () => true, + onCancelAndDeleteSession: async () => true, onFunctionResult: async () => undefined, onRefresh: () => undefined, onRetrySession: () => undefined, diff --git a/apps/web/src/features/sessions/SessionsView.tsx b/apps/web/src/features/sessions/SessionsView.tsx index e2d2f9785..cd4091645 100644 --- a/apps/web/src/features/sessions/SessionsView.tsx +++ b/apps/web/src/features/sessions/SessionsView.tsx @@ -102,6 +102,7 @@ interface SessionsViewProps { onCreateEnvironmentTemplate?: (input: CreateEnvironmentTemplateInput) => Promise; onCreateSession: (input: SessionStartInput) => Promise; onDeleteSession: (sessionId: string) => Promise; + onCancelAndDeleteSession: (sessionId: string) => Promise; onFunctionResult: (input: FunctionResultInput) => Promise; onListEnvironmentFiles?: ListEnvironmentFiles; onCreateEnvironmentFile?: AgentCore["createEnvironmentFile"]; @@ -305,6 +306,7 @@ export function SessionsView({ onCreateEnvironmentTemplate, onCreateSession, onDeleteSession, + onCancelAndDeleteSession, onFunctionResult, onListEnvironmentFiles, onCreateEnvironmentFile, @@ -1000,6 +1002,7 @@ export function SessionsView({ session={actionSession} onClose={() => setActionSession(null)} onDelete={onDeleteSession} + onCancelAndDelete={onCancelAndDeleteSession} onDeleted={(sessionId) => { draftsBySessionRef.current.delete(sessionId); if (selectedIdRef.current === sessionId) setMessage(""); diff --git a/apps/web/src/features/sessions/actions/SessionActionsDialog.test.tsx b/apps/web/src/features/sessions/actions/SessionActionsDialog.test.tsx index dc495ec59..8660a83c5 100644 --- a/apps/web/src/features/sessions/actions/SessionActionsDialog.test.tsx +++ b/apps/web/src/features/sessions/actions/SessionActionsDialog.test.tsx @@ -71,5 +71,14 @@ describe("Session actions dialog content", () => { expect(html).toContain("server lifecycle semantics"); expect(html).toContain("not a promise of physical history erasure"); expect(html).toContain("Workspace files"); + expect(html).not.toContain("Cancel work and delete"); + }); + + it("explains cancel-then-delete only after Core reports a busy Session", () => { + const html = renderToStaticMarkup(); + expect(html).toContain("Cancel work and delete sends one cancellation"); + expect(html).toContain("waits until Core reports the Session idle or failed"); + expect(html).toContain("then sends one deletion"); + expect(html).toContain("cannot be cancelled"); }); }); diff --git a/apps/web/src/features/sessions/actions/SessionActionsDialog.tsx b/apps/web/src/features/sessions/actions/SessionActionsDialog.tsx index 69f536038..734db26a6 100644 --- a/apps/web/src/features/sessions/actions/SessionActionsDialog.tsx +++ b/apps/web/src/features/sessions/actions/SessionActionsDialog.tsx @@ -23,6 +23,7 @@ interface SessionActionsDialogProps { session: AgentSession | null; onClose: () => void; onDelete: (sessionId: string) => Promise; + onCancelAndDelete: (sessionId: string) => Promise; onDeleted: (sessionId: string) => void; onRetrieve: (sessionId: string) => Promise; onUpdate: ( @@ -69,13 +70,16 @@ export function SessionDetails({ session }: { session: AgentSession }) { ); } -export function SessionDeleteConfirmation({ session }: { session: AgentSession }) { +export function SessionDeleteConfirmation({ session, busy = false }: { session: AgentSession; busy?: boolean }) { return (

Delete {sessionTitle(session)} from Agent Core?

Exact Session: {session.id}

The Web removes this Session only after Core confirms success. A missing, conflicting, unavailable, or uncertain response leaves the current durable view in place and is never retried automatically.

Parsar deletion follows server lifecycle semantics. It is not a promise of physical history erasure, immediate native executor shutdown, or deletion of executor Workspace files.

+ {busy ? ( +

Cancel work and delete sends one cancellation for the current work, waits until Core reports the Session idle or failed, and then sends one deletion. Input still waiting for its Environment cannot be cancelled; wait for it to start or expire.

+ ) : null}
); } @@ -151,6 +155,7 @@ export function SessionActionsDialog({ session, onClose, onDelete, + onCancelAndDelete, onDeleted, onRetrieve, onUpdate, @@ -160,6 +165,8 @@ export function SessionActionsDialog({ const [detailLoading, setDetailLoading] = useState(false); const [pending, setPending] = useState(false); const [deleteRetryBlocked, setDeleteRetryBlocked] = useState(false); + // Core refused deletion because the Session still has work or pending input. + const [deleteBusy, setDeleteBusy] = useState(false); const [actionError, setActionError] = useState(null); const [formReplacement, setFormReplacement] = useState<{ revision: number; @@ -180,6 +187,7 @@ export function SessionActionsDialog({ setActionError(null); setFormReplacement(null); setPending(false); + setDeleteBusy(false); setDeleteRetryBlocked(uncertainDeleteSessionRef.current === session?.id); setDetailLoading(Boolean(session)); if (!session) return; @@ -227,6 +235,7 @@ export function SessionActionsDialog({ const returnToDetail = () => { if (pending) return; if (!deleteRetryBlocked) setActionError(null); + setDeleteBusy(false); restoreDetailFocus.current = true; setMode("detail"); }; @@ -264,14 +273,14 @@ export function SessionActionsDialog({ } }; - const confirmDelete = async () => { + const confirmDelete = async (cancelFirst = false) => { if (!current || pending) return; const request = requestRef.current + 1; requestRef.current = request; setActionError(null); setPending(true); try { - const confirmed = await onDelete(current.id); + const confirmed = await (cancelFirst ? onCancelAndDelete : onDelete)(current.id); if (request !== requestRef.current) return; if (!confirmed) { throw new Error("The deletion confirmation belongs to an earlier Core connection. The current Core view and draft were kept."); @@ -284,6 +293,7 @@ export function SessionActionsDialog({ uncertainDeleteSessionRef.current = current.id; setDeleteRetryBlocked(true); } + if (error instanceof SessionActionError && error.kind === "session_busy") setDeleteBusy(true); setActionError(errorMessage(error)); } } finally { @@ -309,9 +319,15 @@ export function SessionActionsDialog({ ) : mode === "delete" ? ( <> - + {deleteBusy ? ( + + ) : ( + + )} ) : ( <> @@ -340,7 +356,7 @@ export function SessionActionsDialog({ replacement={formReplacement} /> ) : null} - {current && mode === "delete" ? : null} + {current && mode === "delete" ? : null} ); diff --git a/apps/web/src/features/sessions/actions/session-actions.test.ts b/apps/web/src/features/sessions/actions/session-actions.test.ts index 3ec8d253a..ac7574f52 100644 --- a/apps/web/src/features/sessions/actions/session-actions.test.ts +++ b/apps/web/src/features/sessions/actions/session-actions.test.ts @@ -14,11 +14,13 @@ import { requestSessionUpdate, rebaseSessionMetadataDraft, replaceSessionMetadata, + requestSessionCancelBeforeDelete, selectionAfterSessionDelete, SessionActionError, SessionMetadataConflictError, validateSessionMetadata, valuesFromSession, + waitForSessionIdle, } from "./session-actions"; function session(id: string, metadata: Record = {}): AgentSession { @@ -265,6 +267,8 @@ describe("Session metadata reconciliation", () => { }); }); +const busyMessage = "session must be durably idle or failed without required actions before deletion"; + describe("Session deletion", () => { it("sends one delete, validates confirmation, and classifies 404, 409, 503, and network failures", async () => { const deleteSession = vi.fn().mockResolvedValue({ @@ -278,13 +282,93 @@ describe("Session deletion", () => { for (const [error, kind] of [ [new AgentCoreError("missing", 404), "not_found"], [new AgentCoreError("active conflict", 409), "lifecycle_conflict"], + [new AgentCoreError(busyMessage, 409, "conflict_error", null, "conflict_error"), "session_busy"], [new AgentCoreError("unavailable", 503), "unknown_write"], [new TypeError("connection lost"), "unknown_write"], ] as const) { deleteSession.mockRejectedValueOnce(error); await expect(requestSessionDelete(core, "session-1")).rejects.toMatchObject({ kind }); } - expect(deleteSession).toHaveBeenCalledTimes(5); + expect(deleteSession).toHaveBeenCalledTimes(6); + }); + + it("explains a busy-Session conflict and offers cancel-then-delete without retrying", async () => { + const deleteSession = vi.fn().mockRejectedValue( + new AgentCoreError(busyMessage, 409, "conflict_error", null, "conflict_error"), + ); + const cancelTurn = vi.fn(); + const core = { deleteSession, cancelTurn } as unknown as AgentCore; + const error = await requestSessionDelete(core, "session-1").catch((value: unknown) => value); + expect(error).toBeInstanceOf(SessionActionError); + expect((error as SessionActionError).message).toContain("only when it is idle or failed without required actions"); + expect((error as SessionActionError).message).toContain("nothing was changed"); + expect((error as SessionActionError).message).toContain("Cancel work and delete"); + expect(deleteSession).toHaveBeenCalledTimes(1); + expect(cancelTurn).not.toHaveBeenCalled(); + }); + + it("sends one explicit cancellation before delete and keeps the Session on failure", async () => { + const cancelTurn = vi.fn().mockResolvedValue(undefined); + const deleteSession = vi.fn(); + const core = { cancelTurn, deleteSession } as unknown as AgentCore; + await requestSessionCancelBeforeDelete(core, "session-1", "cancel-key"); + expect(cancelTurn).toHaveBeenCalledWith("session-1", "cancel-key"); + + for (const [error, kind, text] of [ + [new AgentCoreError("pending input", 409, "turn_conflict"), "request_failed", "rejected the cancellation (409)"], + [new TypeError("connection lost"), "request_failed", "result is unknown"], + [new AgentCoreError("missing", 404), "not_found", "not found"], + ] as const) { + cancelTurn.mockRejectedValueOnce(error); + const failure = await requestSessionCancelBeforeDelete(core, "session-1", "cancel-key").catch((value: unknown) => value); + expect(failure).toMatchObject({ kind }); + expect((failure as Error).message).toContain(text); + } + expect(cancelTurn).toHaveBeenCalledTimes(4); + expect(deleteSession).not.toHaveBeenCalled(); + }); + + it("reads until the Session is idle or failed without required actions", async () => { + const pendingAction = { + ...session("session-1"), + status: "requires_action" as const, + required_actions: [{ type: "function_call" as const, call_id: "call-1", turn_id: "turn-1", name: "lookup", arguments: {} }], + }; + const retrieveSession = vi.fn() + .mockResolvedValueOnce({ ...session("session-1"), status: "in_progress" }) + .mockResolvedValueOnce(pendingAction) + .mockResolvedValueOnce(session("session-1")) + .mockResolvedValueOnce({ ...session("session-1"), status: "failed", error: "The execution could not complete." }); + const sleep = vi.fn().mockResolvedValue(undefined); + const core = { retrieveSession, deleteSession: vi.fn(), cancelTurn: vi.fn() } as unknown as AgentCore; + + await expect(waitForSessionIdle(core, "session-1", { sleep, intervalMs: 5 })).resolves.toBe("idle"); + expect(sleep).toHaveBeenCalledTimes(2); + expect(sleep).toHaveBeenCalledWith(5); + await expect(waitForSessionIdle(core, "session-1", { sleep })).resolves.toBe("idle"); + expect(retrieveSession).toHaveBeenCalledTimes(4); + + retrieveSession.mockRejectedValueOnce(new AgentCoreError("missing", 404)); + await expect(waitForSessionIdle(core, "session-1", { sleep })).resolves.toBe("missing"); + retrieveSession.mockRejectedValueOnce(new AgentCoreError("unavailable", 503)); + await expect(waitForSessionIdle(core, "session-1", { sleep })).rejects.toMatchObject({ kind: "request_failed" }); + await expect(waitForSessionIdle(core, "session-1", { sleep, isCurrent: () => false })).resolves.toBe("stale"); + expect(retrieveSession).toHaveBeenCalledTimes(6); + expect((core.deleteSession as ReturnType)).not.toHaveBeenCalled(); + expect((core.cancelTurn as ReturnType)).not.toHaveBeenCalled(); + }); + + it("stops waiting at the deadline without deleting", async () => { + let clock = 0; + const retrieveSession = vi.fn().mockResolvedValue({ ...session("session-1"), status: "in_progress" }); + const sleep = vi.fn(async (ms: number) => { clock += ms; }); + const core = { retrieveSession } as unknown as AgentCore; + const failure = await waitForSessionIdle(core, "session-1", { + sleep, now: () => clock, intervalMs: 1_000, timeoutMs: 3_000, + }).catch((value: unknown) => value); + expect(failure).toMatchObject({ kind: "session_busy" }); + expect((failure as Error).message).toContain("still in progress after 3 seconds"); + expect(retrieveSession).toHaveBeenCalledTimes(4); }); it("keeps the Session when Core returns a malformed confirmation", async () => { diff --git a/apps/web/src/features/sessions/actions/session-actions.ts b/apps/web/src/features/sessions/actions/session-actions.ts index 608177342..6b6701b57 100644 --- a/apps/web/src/features/sessions/actions/session-actions.ts +++ b/apps/web/src/features/sessions/actions/session-actions.ts @@ -1,5 +1,6 @@ import { AgentCoreError, + isSessionDeletionConflict, type AgentCore, type AgentSession, type SessionDeleted, @@ -18,6 +19,7 @@ export interface SessionMetadataValidation { export type SessionActionFailureKind = | "not_found" | "lifecycle_conflict" + | "session_busy" | "core_unavailable" | "metadata_conflict" | "request_failed" @@ -295,6 +297,13 @@ function normalizeSessionActionError( { cause: error }, ); } + if (action === "delete" && phase === "write" && isSessionDeletionConflict(error)) { + return new SessionActionError( + "Agent Core deletes a Session only when it is idle or failed without required actions. This Session still has queued, running or waiting work or pending input, so nothing was changed. Choose Cancel work and delete to cancel it, wait until it is idle and then delete it.", + "session_busy", + { cause: error }, + ); + } if (error.status === 409) { if (phase === "read") { return new SessionActionError( @@ -409,6 +418,88 @@ export async function requestSessionDelete(core: AgentCore, sessionId: string): return deleted; } +/** + * Sends one explicit cancellation before a confirmed delete. It never deletes; + * a rejected or uncertain cancellation keeps the Session. + */ +export async function requestSessionCancelBeforeDelete( + core: AgentCore, + sessionId: string, + idempotencyKey: string, +): Promise { + try { + await core.cancelTurn(sessionId, idempotencyKey); + } catch (error) { + if (error instanceof AgentCoreError && error.status === 404) { + throw normalizeSessionActionError(error, "delete", "write"); + } + const detail = error instanceof AgentCoreError + ? `Agent Core rejected the cancellation (${error.status}): ${error.message}` + : "The cancellation result is unknown because the connection ended before Core confirmed it."; + throw new SessionActionError( + `${detail} The Session was not deleted and the Web did not retry.`, + "request_failed", + { cause: error }, + ); + } +} + +export type SessionIdleWait = "idle" | "missing" | "stale"; + +export interface SessionIdleWaitOptions { + /** Stops waiting without error once the result would no longer be applied. */ + isCurrent?: () => boolean; + intervalMs?: number; + timeoutMs?: number; + now?: () => number; + sleep?: (ms: number) => Promise; +} + +function isDeletableSession(session: AgentSession): boolean { + return (session.status === "idle" || session.status === "failed") && session.required_actions.length === 0; +} + +/** + * Reads the Session until it is idle or failed without required actions, the + * public state that permits deletion. The wait is bounded and never writes. + */ +export async function waitForSessionIdle( + core: AgentCore, + sessionId: string, + { + isCurrent = () => true, + intervalMs = 1_000, + timeoutMs = 30_000, + now = Date.now, + sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)), + }: SessionIdleWaitOptions = {}, +): Promise { + const deadline = now() + timeoutMs; + for (;;) { + if (!isCurrent()) return "stale"; + let latest: AgentSession; + try { + latest = await retrieveCanonicalSession(core, sessionId); + } catch (error) { + if (error instanceof AgentCoreError && error.status === 404) return "missing"; + throw new SessionActionError( + "Cancellation was requested, but the Session could not be read while waiting for it to become idle. It was not deleted.", + "request_failed", + { cause: error }, + ); + } + if (!isCurrent()) return "stale"; + if (isDeletableSession(latest)) return "idle"; + if (now() >= deadline) { + throw new SessionActionError( + `Cancellation was requested, but the Session was still ${latest.status.replaceAll("_", " ")} after ${Math.round(timeoutMs / 1000)} seconds. It was not deleted; try again once it is idle.`, + "session_busy", + ); + } + await sleep(intervalMs); + } +} + export async function reconcileUnknownSessionDelete( core: AgentCore, sessionId: string, diff --git a/docs/web/protocol-coverage.md b/docs/web/protocol-coverage.md index 0bbf40086..3d070c0b2 100644 --- a/docs/web/protocol-coverage.md +++ b/docs/web/protocol-coverage.md @@ -513,9 +513,16 @@ upstream. Environment observation, send failure, and draft, then selects the next item at the deleted position or the previous item at the end. Deleting an inactive Session does not change the selected ID, stream epoch, composer, or current conversation state. +- Core deletes only a durably idle or failed Session without required actions or + pending input; otherwise it returns 409 `conflict_error` and changes nothing. The + dialog then explains the conflict and replaces its action with Cancel work and + delete. Only that explicit choice sends one `agent.session.input.cancel`, reads the + Session until it is idle or failed without required actions (at most 30 seconds) + and sends one deletion. A rejected or uncertain cancellation, a timeout, a + connection change or another 409 stops without a retry; other 409 responses keep + the generic lifecycle-conflict message. - Parsar deletion is a public server lifecycle operation. It hides the durable public - Session/Items/Turns, closes its stream, cancels queued work, and requests asynchronous - cancellation of active work. It does not prove immediate native executor quiescence, + Session/Items/Turns and closes its stream. It does not prove immediate native executor quiescence, physical SQL/native-history erasure, or deletion of executor Workspace files. ## Live stream and recovery diff --git a/packages/agents-client/src/client.test.ts b/packages/agents-client/src/client.test.ts index 46ac55a5f..41ccec5f0 100644 --- a/packages/agents-client/src/client.test.ts +++ b/packages/agents-client/src/client.test.ts @@ -1,6 +1,6 @@ import { afterEach, describe, expect, it, vi } from "vitest"; -import { AgentCoreError, CreationStreamRetryError, createIdempotencyKey, OpenAIAgentsClient } from "./client"; +import { AgentCoreError, CreationStreamRetryError, createIdempotencyKey, isSessionDeletionConflict, OpenAIAgentsClient } from "./client"; import hostedDadf64 from "./fixtures/parsar-dadf64a7/openai-hosted.json"; import eventBatchDadf64 from "./fixtures/parsar-dadf64a7/session-event-batch.json"; import type { @@ -219,6 +219,30 @@ describe("OpenAIAgentsClient", () => { } }); + it("surfaces the busy-Session deletion conflict with its official fields", async () => { + const calls: FetchCall[] = []; + const message = "session must be durably idle or failed without required actions before deletion"; + const client = new OpenAIAgentsClient({ + baseUrl: "https://core.example/v1", + token: "tenant-key", + fetch: recordingFetch(jsonResponse({ + error: { type: "conflict_error", code: "conflict_error", message, param: null }, + }, 409), calls), + }); + + const error = await client.deleteSession("session_busy").catch((value: unknown) => value); + expect(error).toBeInstanceOf(AgentCoreError); + expect(error).toMatchObject({ status: 409, code: "conflict_error", errorType: "conflict_error", param: null, message }); + expect(isSessionDeletionConflict(error)).toBe(true); + expect(calls).toHaveLength(1); + expect(calls[0]?.init?.method).toBe("DELETE"); + + expect(isSessionDeletionConflict(new AgentCoreError("Resource not found.", 404, "not_found_error"))).toBe(false); + expect(isSessionDeletionConflict(new AgentCoreError("Other conflict.", 409, "turn_conflict"))).toBe(false); + expect(isSessionDeletionConflict(new CreationStreamRetryError())).toBe(false); + expect(isSessionDeletionConflict(new Error(message))).toBe(false); + }); + it("preserves the event-stream Accept header and decodes streamed events", async () => { const calls: FetchCall[] = []; const encoder = new TextEncoder(); diff --git a/packages/agents-client/src/client.ts b/packages/agents-client/src/client.ts index d57e3b491..a28516be5 100644 --- a/packages/agents-client/src/client.ts +++ b/packages/agents-client/src/client.ts @@ -118,6 +118,16 @@ export class CreationStreamRetryError extends AgentCoreError { } } +/** + * Core deletes only a durably idle or failed Session without required actions + * or pending input. Any other Session is rejected with HTTP 409 and code + * `conflict_error` and left unchanged: cancel its work, wait until it is idle, + * then delete it. + */ +export function isSessionDeletionConflict(error: unknown): error is AgentCoreError { + return error instanceof AgentCoreError && error.status === 409 && error.code === "conflict_error"; +} + function trimTrailingSlash(value: string): string { return value.replace(/\/+$/, ""); } @@ -2466,6 +2476,11 @@ export class OpenAIAgentsClient implements AgentCore { return projectAgentSession(value, undefined, sessionId); } + /** + * Deletes a durably idle or failed Session. The owner's repeated deletion of + * a deleted Session returns the same confirmation. A busy Session rejects with + * an error matched by `isSessionDeletionConflict`. + */ deleteSession(sessionId: string): Promise { return this.request(`/agents/sessions/${encodeURIComponent(sessionId)}`, { method: "DELETE" }); } diff --git a/packages/agents-client/src/index.ts b/packages/agents-client/src/index.ts index 87a999095..20dd3eaf1 100644 --- a/packages/agents-client/src/index.ts +++ b/packages/agents-client/src/index.ts @@ -1,4 +1,4 @@ -export { AgentCoreError, CreationStreamRetryError, createIdempotencyKey, OpenAIAgentsClient } from "./client"; +export { AgentCoreError, CreationStreamRetryError, createIdempotencyKey, isSessionDeletionConflict, OpenAIAgentsClient } from "./client"; export type { OpenAIAgentsClientOptions } from "./client"; export { createSSEDecoder } from "./sse"; export type { SSEDecoder, SSEMessage } from "./sse"; From e1fcd8a0247557a580c28867e02c60d5c22527ee Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 12:39:43 +0000 Subject: [PATCH 3/8] Record the Session deletion lifecycle alignment Document rows D1-D5, the lock-ordered decision, legacy deletion markers and the stricter local 409 right after an events.create 202 where the official service returned 200. Update the contributor rules, contract README, Environment reservation note and operation evidence row 10. --- CONTRIBUTING.md | 45 ++++++++----- contracts/agents-api/README.md | 17 ++--- contracts/agents-api/environments.md | 5 +- contracts/agents-api/list-query-semantics.md | 3 +- .../official-semantics-alignment.md | 63 +++++++++++++++++++ contracts/agents-api/operation-evidence.md | 5 +- 6 files changed, 111 insertions(+), 27 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e701a6ba8..c44697f65 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -32,7 +32,9 @@ The Core Web is an administrator console for execution and resource operations; business collaboration remains in Parsar. Environment Template management shares the Session creation catalog and uses the existing public client operations. Patch only edited fields, confirm deletion, and never automatically retry an uncertain -write. A Core connection change must discard the previous connection's forms, +write. When Core refuses to delete a busy Session, offer an explicit Cancel work +and delete action that cancels once, reads until the Session is idle within a +bounded wait and deletes once; never cancel without that confirmation. A Core connection change must discard the previous connection's forms, pending results and notices. Saving a Template must not allocate a Runtime, call a model or imply execution readiness. Keep unsupported advanced profiles explicit. @@ -1144,7 +1146,7 @@ direct batches, including cancellation, while successful earlier retries remain readable. Promotion commits the original inputs, history, reservation settlement and execution claim (`queued` to `in_progress`) together; expiration and targeted cancellation retain the terminal identity. Session deletion -cancels pending input in the same transaction. A terminal reservation retry must not +is rejected while input is pending and changes nothing. A terminal reservation retry must not affect a later reservation or Turn. Evaluate deadlines after acquiring the Session lock, and return terminal storage outcomes without rolling their transaction back. @@ -1202,7 +1204,7 @@ An admitted retry returns the original receipts without reclaiming execution; a read or uncertain commit never authorizes another Start. A crash after promotion but before Start uses existing claimed-Turn reconciliation (`execution_interrupted`), including unbound or deleted Sessions, rather than ordinary queued dispatch. Deletion -after claim requests cancellation under existing active-Turn semantics. +after claim is rejected like any active Turn. The Worker expires at most 32 due reservations on each existing tick, after checking ownership and before checking devices or execution slots. The sweep requires the leased Store and uses its connection with the existing transaction @@ -1643,21 +1645,34 @@ replaced; do not carry obsolete compatibility code forward to satisfy this secti types. Product adapters live in `server/internal/agentdaemon`. Keep protocol frames in `internal/agentdaemon/proto` until the contracts directory migration. Store aliases preserve existing callers during this transition. -- Session deletion uses a durable `sessions.deleted_at` marker, committed with an - existing Turn cancellation request under the tenant Session lock. Public reads, - metadata changes, event streams and input admission exclude deleted Sessions; - admission checks visibility under that lock before retry lookup. Creation keys - remain reserved and cannot resurrect deleted Sessions. Missing/repeated deletion - locally returns 404 and reuse of a deleted creation identity returns 409; exact - hosted errors and overlapping stream timing remain unverified. Existing streams - close when removal is observed without a fabricated deletion event. +- Session deletion uses a durable `sessions.deleted_at` marker. Public deletion + accepts only a durably idle or failed Session without required actions: no + queued, in-progress or waiting Turn and no pending input reservation, the same + settlement rule as the creation stream. Take that decision and commit the marker + under the tenant Session lock that orders Turn and input admission, so either + admission commits first and deletion conflicts, or admission observes the + deletion. A busy Session returns 409 `conflict_error` with the observed official + message and nothing changes: no cancellation, marker, event or cleanup. Callers + cancel first (`agent.session.input.cancel`), wait until the Session is idle and + delete it. Core admits a Turn synchronously, so it also conflicts right after an + `events.create` 202, where the official service was observed to return 200. + The owner's repeated deletion returns the same 200 confirmation without writing; + foreign, missing and malformed identifiers keep the byte-identical 404. Public + reads, metadata changes, event streams and input admission exclude deleted + Sessions; admission checks visibility under that lock before retry lookup. + Creation keys remain reserved and cannot resurrect deleted Sessions; reuse of a + deleted creation identity returns 409. Existing streams close when removal is + observed without a fabricated deletion event; overlapping stream timing remains + unverified. Earlier releases also deleted busy Sessions after requesting + cancellation, so upgraded databases can hold markers with hidden work. Internal Turn/receipt/finalization and restart reconciliation retain access so - hidden work can settle under the existing execution lease. Queued deletion - prevents claim; an already claimed execution may complete or receive cancellation. - Confirmation does not guarantee native quiescence. Never revoke a shared device, + that work settles under the existing execution lease; queued work cannot be + claimed, and Runtime cleanup still cancels pending work. Confirmation does not + guarantee native quiescence. Never revoke a shared device, remove a saved Agent or touch product data as part of Session deletion. Physical SQL/native history cleanup remains a separate required implementation gap; these - records are retained, not claimed purged. Do not deploy a pre-deletion service + records are retained, not claimed purged, and purging may end repeat idempotency. + Do not deploy a pre-deletion service against a database with deletion markers; migration rollback refuses to remove the column while deleted records exist, preventing public resurrection. - `services/agents-api` owns its SQL schema, sqlc queries and embedded goose diff --git a/contracts/agents-api/README.md b/contracts/agents-api/README.md index 1c8dc913b..5e8bd559d 100644 --- a/contracts/agents-api/README.md +++ b/contracts/agents-api/README.md @@ -94,7 +94,7 @@ paths start at `/vaults`, not `/agents/vaults`. | --- | --- | --- | | Root reusable Agents | create, retrieve, update, list, delete | Partial create/retrieve/update/list/delete and Session references; configuration/error gaps remain | | Skills and Versions | create, retrieve, update default, list, delete, content | [Tenant-owned encrypted bundles and hosted references](environment-templates.md); [default metadata/content and deletion evidence](file-resource-semantics.md), qualified upload limits and unresolved semantics | -| sessions | create, retrieve, update, list, delete | Create (ordinary/live), retrieve, list with root-Agent filter, metadata-only update, public deletion with owned Docker cleanup; user-managed compute stays caller-owned; general physical cleanup and exact hosted semantics remain open | +| sessions | create, retrieve, update, list, delete | Create (ordinary/live), retrieve, list with root-Agent filter, metadata-only update, [idle-only public deletion](official-semantics-alignment.md#session-deletion-lifecycle--september-23) with idempotent owner repeat and owned Docker cleanup; user-managed compute stays caller-owned; general physical cleanup and exact hosted semantics remain open | | sessions.events | create, stream | Text/cancel/function-result admission and live events; function-action state snapshots supported | | sessions.turns | retrieve, list | Implemented reads; lifecycle conformance still partial | | sessions.items | list | Partial Item variants | @@ -304,13 +304,16 @@ upgrade the protocol. hosted nested/null behavior, no-op timestamp policy and exact errors remain unverified. This operation shares the existing saved-configuration coverage gaps. - `DELETE /agents/sessions/{session_id}` returns the canonical `id`, - `object=agent.session.deleted` and `deleted=true` after durable public removal. - Session/Turn/Items reads, live streams, metadata updates and new input exclude - the resource. Queued work is cancelled; active work receives the existing - asynchronous cancellation request while internal finalization remains available. + `object=agent.session.deleted` and `deleted=true` after durable public removal + of a durably idle or failed Session without required actions or pending input. + A queued, running or waiting Turn or pending input returns 409 `conflict_error` + without any change; callers cancel first and delete once idle. Session/Turn/Items + reads, live streams, metadata updates and new input exclude the resource. Existing streams close on observing removal without an invented deletion event. - Creation keys remain reserved (local 409); missing/repeated deletion locally - returns 404. Qualified managed Docker deletion also reclaims its owned Runtime; + Creation keys remain reserved (local 409); the owner's repeated deletion returns + the same confirmation and missing or foreign deletion returns 404 ([batch + record](official-semantics-alignment.md#session-deletion-lifecycle--september-23)). + Qualified managed Docker deletion also reclaims its owned Runtime; broader physical SQL/native history cleanup, immediate native quiescence and exact hosted error/retry/overlapping-stream semantics remain unverified or unimplemented. Shared devices, saved Agents and other Sessions are independent. diff --git a/contracts/agents-api/environments.md b/contracts/agents-api/environments.md index 210f98f35..b70da1899 100644 --- a/contracts/agents-api/environments.md +++ b/contracts/agents-api/environments.md @@ -395,8 +395,9 @@ requires the leased writer and atomically creates history, settles the reservati and claims its Turn as `in_progress`. Only the first non-replay receipts authorize Start on the retained native preparation. Retries cannot reclaim execution. Startup reconciliation settles a committed claim interrupted before Start, without replay. -Expiration, targeted cancellation and Session deletion retain their existing -pre-admission or claimed-Turn semantics. +Expiration and targeted cancellation retain their existing pre-admission or +claimed-Turn semantics; Session deletion is rejected while a reservation is +pending or its Turn is claimed. The initial public idle-text profile uses this primitive. Its message-only scope and single pending reservation are implementation limits, not claims about the diff --git a/contracts/agents-api/list-query-semantics.md b/contracts/agents-api/list-query-semantics.md index bf93e6351..395478410 100644 --- a/contracts/agents-api/list-query-semantics.md +++ b/contracts/agents-api/list-query-semantics.md @@ -169,7 +169,8 @@ These remain registered differences and are not changed here: repeated Files `purpose` values (SFT-18); unsampled overflowing limits; Skill sole-version deletion and number reuse (SFT-01/02, since resolved or recorded in [file resource semantics](file-resource-semantics.md#sole-version-deletion--september-23-2026)); -Session deletion lifecycle (SES-29/30); +Session deletion lifecycle (SES-29/30, since addressed by the +[deletion batch](official-semantics-alignment.md#session-deletion-lifecycle--september-23)); whitespace input (SES-01..04); Template network forms (SFT-21/22); and response defaults (VA-11, SES-23/25). Malformed path IDs (SES-28), metadata and name error fields (VA-07/08/09), U+0000 (VA-10) and Template network codes (SFT-20) are diff --git a/contracts/agents-api/official-semantics-alignment.md b/contracts/agents-api/official-semantics-alignment.md index 97f5bf551..548a5c38f 100644 --- a/contracts/agents-api/official-semantics-alignment.md +++ b/contracts/agents-api/official-semantics-alignment.md @@ -274,3 +274,66 @@ cover the envelope, other, foreign and malformed filters, and foreign or missing Sessions. The pinned-SDK and raw HTTP verifier used by live acceptance runs against PostgreSQL across three Turns. Real Core, daemon and model acceptance is recorded separately by the coordinator. + +## Session deletion lifecycle — September 23 + +This batch aligns Session deletion with the observed official lifecycle rules. +Evidence comes from the campaign scan at main `beb18fd`, recorded privately in +`~/.parsar/remediation/20260923/campaign-scan-1/sessions/findings.json` (SES-29 +and SES-30) with raw records under `official/`: `q5-delete-repeat.json`, +`q5b-delete-while-in-progress.json`, `q5-delete-never-existed.json` and +`q5-delete-while-running.json`, plus the September 22 retry-session cleanup that +first returned 409. + +| Row | Case | Core behavior | +| --- | --- | --- | +| D1 | DELETE of the caller's own Session that is already publicly deleted (SES-29) | 200 `{id, object: "agent.session.deleted", deleted: true}`, identical to the first confirmation, with no database write. GET, update, events, Turns and Items stay 404. | +| D2 | DELETE of a never-existing, malformed or foreign Session, including a foreign deleted one | Unchanged: the byte-identical 404 `not_found_error` of a missing Session. | +| D3 | DELETE while a Turn is queued, in progress (including a requested cancellation) or waiting on required actions or function results, or while an input reservation is pending: a queued later input, self-hosted input awaiting a connection, or hosted initial input while provisioning (SES-30) | 409 with type and code `conflict_error`, param null and message "session must be durably idle or failed without required actions before deletion". Nothing changes: no cancellation, marker, event, Artifact removal or Runtime cleanup. | +| D4 | DELETE of an idle Session, including an idle hosted Session still provisioning without input, and of a failed Session without required actions, including expired initial input | 200 with the existing public deletion and managed Runtime cleanup. | +| D5 | Callers that need to delete running work | Cancel first with `agent.session.input.cancel`, wait until the Session is idle, then delete. The Core Web offers this as an explicit action after a 409. | + +Decisions: + +- The rule is the one the creation stream already uses to settle: the Session is + idle or failed, no Turn is queued, running or waiting, and the latest input + reservation is not pending. Deletion reuses the Store's active-Turn query and + reservation state, so a pending reservation blocks deletion even while the + public status projects idle. +- The decision and the marker commit in one transaction under the tenant Session + row lock that also orders Turn and input admission. Either admission commits + first and deletion returns 409 without mutation, or deletion commits first and + admission returns 404. A rejected deletion rolls its transaction back. +- A repeated deletion locks the owner's deleted row and returns the confirmation + without a write. Foreign and missing rows are never locked, so they stay + indistinguishable. Physical purge, when implemented, may end this idempotency. +- Documented stricter local behavior: official DELETE immediately after an + `events.create` 202 on an idle Session returned 200 (`q5-delete-while-running`); + its Turn was apparently not yet durably in progress. Core admits the Turn + synchronously in the 202 transaction, so Core returns 409 in that window. +- A self-hosted or hosted Session whose reserved input waits for its Environment + cannot be cancelled publicly (the pending reservation rejects new batches), so + it stays undeletable until the input starts, its five-minute deadline expires + or its Environment fails. The official behavior of that window is unobserved. +- Earlier releases deleted busy Sessions after requesting cancellation. Their + markers can remain in upgraded databases; hidden-work settlement, restart + reconciliation and Runtime cleanup keep handling them unchanged. +- The Core Web keeps the plain delete action. When Core returns the busy 409, the + dialog explains it and replaces the action with Cancel work and delete, which + sends one cancellation, reads the Session until it is idle or failed without + required actions (bounded to 30 seconds) and sends one deletion. A rejected or + uncertain cancellation, a timeout or another 409 stops without retrying. + +Unchanged: physical retention and purge (SESSION-CLEANUP-001 remainder), 404 for +reads of deleted Sessions, Artifact retention rules after deletion, managed Runtime +cleanup once deletion is allowed, and caller-owned self-hosted compute, which is +never reclaimed. No schema change. + +Real-PostgreSQL HTTP tests replay D1–D4 across every busy and settled state with +exact bodies, tenant B requests and a whole-database digest proving that a 409 +and a repeated deletion write nothing. Store tests race deletion against Turn and +input admission on one real row lock in both commit orders and concurrently, and +a Worker test cancels a waiting Turn through the daemon protocol before deleting. +Handler, pinned-SDK, TypeScript client and Web unit tests cover the error fields +and the cancel-then-delete flow. Real Core, daemon and model acceptance is +recorded separately by the coordinator. diff --git a/contracts/agents-api/operation-evidence.md b/contracts/agents-api/operation-evidence.md index abf293713..cf2744c44 100644 --- a/contracts/agents-api/operation-evidence.md +++ b/contracts/agents-api/operation-evidence.md @@ -1,6 +1,6 @@ # Pinned operation evidence inventory — 2026-09-23 -Baseline inventory of main `b5715912f09333e2b4449ec6f0eecaabce44c9b7`. The Session admission batch below updates creation and metadata validation, the list query tolerance batch (L) updates list and resource query handling, the validation error batch (X) updates field error codes/params, malformed path IDs and U+0000 handling, the Environment Files wire batch (G) updates Files.create/list status, envelope, query, path and empty-page behavior, and the creation stream settlement batch (J) updates creation SSE lifetime/snapshot, terminal Turn usage and Turn start order; historical evidence retains its original revision and scope. This inventory guides repeated qualification and does not assert complete compatibility. +Baseline inventory of main `b5715912f09333e2b4449ec6f0eecaabce44c9b7`. The Session admission batch below updates creation and metadata validation, the list query tolerance batch (L) updates list and resource query handling, the validation error batch (X) updates field error codes/params, malformed path IDs and U+0000 handling, the Environment Files wire batch (G) updates Files.create/list status, envelope, query, path and empty-page behavior, and the creation stream settlement batch (J) updates creation SSE lifetime/snapshot, terminal Turn usage and Turn start order, and the Session deletion batch (Z) updates the deletion lifecycle; historical evidence retains its original revision and scope. This inventory guides repeated qualification and does not assert complete compatibility. Baseline: `contracts/agents-api/upstream.json`, SDK **3.13.0**, upstream commit **d7c41efee1b0802b79f3f88a678ef2052b06e9ce**, `OpenAI-Beta: agents=v1`. AGENTS.md and relevant CONTRIBUTING.md compatibility, ownership and evidence rules govern this inventory. @@ -41,6 +41,7 @@ Repository paths below are relative to the inspected worktree; private evidence | L | [List query tolerance](list-query-semantics.md#list-query-tolerance--september-23-2026); private `~/.parsar/remediation/20260923/campaign-scan-1/{vaults-agents,sessions,skills-files-templates}/findings.json`: owned-collection unknown/repeated keys, limit bounds, Vault status union and Files empty purpose, plus unknown keys on a deleted Vault read and Agent delete. Rows A1–D2 of that section; no model execution. | | G | [Environment Files wire alignment](environment-files.md#wire-alignment--september-23-2026); private `~/.parsar/remediation/20260923/campaign-scan-2/hosted-env/findings.json` HE-10, 16, 18, 32, 34–39 with raw records under `official/` (labels `fc01`–`fc17`, `fl01`–`fl16`, `files-*-pending`) and `run1/`: three owned hosted Sessions, all deleted; first official Environment Files observations. Rows F1–F9 of that section. Go handler, real-PostgreSQL Worker, gateway/daemon and Rust helper tests without a model; live acceptance is recorded with the batch. | | Y | [Artifact capture and listing](official-semantics-alignment.md#artifact-capture-and-listing--september-23); private `~/.parsar/remediation/20260923/campaign-scan-2/hosted-env/findings.json` HE-50..62 with raw records under `official/` (labels `al01`–`al09`, `ar01`–`ar04`, `ac01`–`ac05`, `ad01`/`ad02`): three owned Sessions and two tiny Turns, all deleted; first official Artifact observations. Rows A1–A4 of that section: symlink skip, republication, list envelope and malformed filter. Rust link tests, real-PostgreSQL store/HTTP and pinned-SDK tests without a model; live acceptance is recorded with the batch. | +| Z | [Session deletion lifecycle](official-semantics-alignment.md#session-deletion-lifecycle--september-23); private `~/.parsar/remediation/20260923/campaign-scan-1/sessions/findings.json` SES-29/30 with raw records under `official/` (`q5-delete-repeat`, `q5b-delete-while-in-progress`, `q5-delete-never-existed`, `q5-delete-while-running`, `q5-get-after-delete`). Rows D1–D5 of that section. Handler, real-PostgreSQL HTTP/store/lock-race, Worker, pinned-SDK, TypeScript client and Web tests without a model; live acceptance is recorded with the batch. | ## Per-operation evidence matrix @@ -57,7 +58,7 @@ Paths in the appendix include `/v1`. SDK names here omit `client.`. `P` means pa | 7 | beta.agents.sessions.retrieve | P: persisted state, required actions, usage; malformed ID equals missing | S `retrieve-1.json`, `session-after-1.json`; H recovered state; X SES-28 | C Live history; T pending actions; H Core acceptance recorded | Complete statuses/actions/lifecycle timing; Claude/MiniMax public usage remains null | | 8 | beta.agents.sessions.update | P: metadata-only replacement/clear; metadata errors use official code and `metadata`/`metadata.` param | S `metadata-replace/null/empty/omit/invalid-value.json`; `update-agent.json` uses newer unpinned field | N Live completed Session metadata rejection/clear/isolation; recorded active controlled metadata coverage | Session admission batch changes empty update to observed official 400; Session agent update belongs to baseline upgrade, not fixed-pin operation gap | | 9 | beta.agents.sessions.list | P: Agent filter, full envelope, cursor paging; unknown keys ignored, limit 0/above 100 clamp | S `list-filter.json`, `list-empty-after.json`, `list-owned-cross-filter-cursor.json`, limit/order/unknown-query samples; L SES-10/11/15/16/17 | C Live order/cursors/empty/tenant checks; L DB tenant A/B | Core page capacity 100; official cap unknown. Eventual visibility sample is not a required delay | -| 10 | beta.agents.sessions.delete | P: public deletion, owned managed cleanup, user compute retained | S cleanup files 200/deleted; retry-session active cleanup initially 409 | D recorded real cleanup; C cleanup separately recorded | Physical purge/retention and all active/unknown-effect races; caller compute ownership preserved | +| 10 | beta.agents.sessions.delete | P: deletion only of a durably idle or failed Session without required actions or pending input; otherwise 409 `conflict_error` with no change; owner repeat returns the same 200; owned managed cleanup, user compute retained | S cleanup files 200/deleted; retry-session active cleanup initially 409; Z SES-29 `q5-delete-repeat` 200, SES-30 `q5b-delete-while-in-progress` 409, `q5-delete-never-existed` 404, `q5-delete-while-running` 200 right after an `events.create` 202 | D recorded real cleanup; C cleanup separately recorded; Z DB matrix D1–D4 with no-write digest, admission lock race and pinned-SDK script | Core returns 409 right after an `events.create` 202 because it admits Turns synchronously (official 200); input awaiting its Environment cannot be cancelled publicly and is unobserved officially; physical purge/retention may end repeat idempotency; caller compute ownership preserved | | 11 | beta.agents.sessions.events.create | P: 202/empty body, empty-array authenticated no-op, text/cancel/function admission | S `second-turn-create.json`, `events-empty/null.json`; H second-input | C Live real continuation/no-op; T qualified message/result/cancel workflows | Mixed prepared-environment batches, native receipt vs durable acceptance, cancel-before-result-publication timing; unqualified content/tools | | 12 | beta.agents.sessions.events.stream | P: live-only SSE, typed persisted projections; terminal Turn events carry top-level Turn usage (null when unknown); new Turns start `turn.created`, user `item.added`, `session.in_progress`, `turn.in_progress` | H create/reconnect frames; no historical frames in sampled idle interval; J GET streams never ended, terminal `usage` present | C Live; H recorded three-harness disconnect/recovery; T pending actions; J DB order/usage | Full SSE/Item variants/order (EVT-05..10 deferred); child deltas settle late; no replay guarantee or observer-disconnect proof for every state | | 13 | beta.agents.sessions.turns.retrieve | P: persisted root/child Turn identity; malformed ID equals missing | H/S contain Turn list payloads; no isolated positive retrieve raw request identified in this set; X SES-28 official `turn_` 404 | Recorded H/B scoped Turn recovery; C history uses list | Distinguish list-shape evidence from retrieve wire qualification; full lifecycle/usage | From 5534f5d95d811467fffc82321058458c18dbb539 Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 13:09:46 +0000 Subject: [PATCH 4/8] Keep native acceptance cleanup from masking failures Session deletion now returns 409 for busy Sessions, so a finally-block delete after a failed assertion could replace the original error. A shared helper cancels once on the 409 conflict_error, polls briefly for idle or failed and deletes again. A cleanup failure is logged while another error propagates and raised otherwise. --- .../tests/official_hosted_functions_native.py | 4 +- .../official_hosted_structured_native.py | 4 +- .../tests/official_pending_actions_native.py | 4 +- .../tests/official_workspace_images_native.py | 5 ++- services/agents-api/tests/session_cleanup.py | 39 +++++++++++++++++++ 5 files changed, 51 insertions(+), 5 deletions(-) create mode 100644 services/agents-api/tests/session_cleanup.py diff --git a/services/agents-api/tests/official_hosted_functions_native.py b/services/agents-api/tests/official_hosted_functions_native.py index 8c7becbc9..3048d4025 100644 --- a/services/agents-api/tests/official_hosted_functions_native.py +++ b/services/agents-api/tests/official_hosted_functions_native.py @@ -6,6 +6,8 @@ import time import uuid +from session_cleanup import delete_session + def verify_hosted_functions(client, foreign, http, model, restart, evidence): """restart(session_id, environment_id) restarts the operator-owned deployment.""" @@ -106,4 +108,4 @@ def failure(arguments): Path(evidence).write_text(json.dumps(proof, indent=2)) return checks finally: - sessions.delete(session.id) + delete_session(sessions, session.id) diff --git a/services/agents-api/tests/official_hosted_structured_native.py b/services/agents-api/tests/official_hosted_structured_native.py index 31d4e77c8..8d07ba5ae 100644 --- a/services/agents-api/tests/official_hosted_structured_native.py +++ b/services/agents-api/tests/official_hosted_structured_native.py @@ -5,6 +5,8 @@ import time import uuid +from session_cleanup import delete_session + def verify_hosted_structured(client, foreign, http, model, kind, restart, evidence): pin = json.loads((Path(__file__).resolve().parents[3] / "contracts/agents-api/upstream.json").read_text()) @@ -203,6 +205,6 @@ def cancel(action): finally: save() for sid in reversed(owned): - sessions.delete(sid) + delete_session(sessions, sid) if saved: client.beta.agents.delete(saved.id) diff --git a/services/agents-api/tests/official_pending_actions_native.py b/services/agents-api/tests/official_pending_actions_native.py index 8acfef084..e6eb54700 100644 --- a/services/agents-api/tests/official_pending_actions_native.py +++ b/services/agents-api/tests/official_pending_actions_native.py @@ -5,6 +5,8 @@ from pathlib import Path import uuid +from session_cleanup import delete_session + def verify_pending_actions(client, foreign, http, model, evidence): """The private runner supplies an isolated Core/native provider deployment.""" @@ -162,4 +164,4 @@ def pending_unchanged(): finally: Path(evidence).write_text(json.dumps(proof, indent=2)) for sid in reversed(created): - sessions.delete(sid) + delete_session(sessions, sid) diff --git a/services/agents-api/tests/official_workspace_images_native.py b/services/agents-api/tests/official_workspace_images_native.py index a1afbe7e9..fa9b4bfe4 100644 --- a/services/agents-api/tests/official_workspace_images_native.py +++ b/services/agents-api/tests/official_workspace_images_native.py @@ -8,6 +8,7 @@ import time from image_fixture import picture +from session_cleanup import delete_session def verify_workspace_images(client, foreign, http, model, kind, restart, evidence): @@ -224,7 +225,7 @@ def active(action): assert history() == before check("same_tenant_session_result_artifact_and_workspace_isolation") finally: - sessions.delete(other.id) + delete_session(sessions, other.id) restart(sid, eid) assert history() == before @@ -250,4 +251,4 @@ def cancel(action): finally: save() if sid: - sessions.delete(sid) + delete_session(sessions, sid) diff --git a/services/agents-api/tests/session_cleanup.py b/services/agents-api/tests/session_cleanup.py new file mode 100644 index 000000000..3560948f3 --- /dev/null +++ b/services/agents-api/tests/session_cleanup.py @@ -0,0 +1,39 @@ +"""Session cleanup for acceptance scripts that must not mask an earlier failure.""" + +import sys +import time + +from openai import APIStatusError + + +def delete_session(sessions, session_id, timeout=60): + """Delete an owned Session from a ``finally`` block. + + Core deletes only a durably idle or failed Session. When deletion returns the + 409 ``conflict_error`` for a busy Session, cancel once, poll until the Session + is idle or failed without required actions, then delete once more. A cleanup + failure is logged while another exception is propagating, so the original + assertion stays visible, and raised otherwise. + """ + original = sys.exc_info()[1] + try: + try: + sessions.delete(session_id) + return + except APIStatusError as exc: + if exc.status_code != 409 or getattr(exc, "code", None) != "conflict_error": + raise + sessions.events.create(session_id, events=[{"type": "agent.session.input.cancel"}]) + deadline = time.monotonic() + timeout + while True: + state = sessions.retrieve(session_id) + if state.status in ("idle", "failed") and not state.required_actions: + break + if time.monotonic() >= deadline: + break + time.sleep(1) + sessions.delete(session_id) + except Exception as exc: + if original is None: + raise + print(f"Session {session_id} cleanup failed after an earlier error: {exc!r}", file=sys.stderr) From 82b8bcde72d21daa517133f789efef2b3ccfe2a7 Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 13:11:13 +0000 Subject: [PATCH 5/8] Explain undeletable pending input without offering cancellation Hosted provisioning with input and a connected self-hosted Session with a queued later input read idle, or await their Environment, while deletion returns 409, and Core rejects cancellation in those states. After a busy conflict the Web reads the Session once and, when only such input remains, explains that it must start, expire or fail first instead of offering Cancel work and delete. Turn-busy Sessions keep the existing flow. --- .../sessions/actions/SessionActionsDialog.tsx | 2 + .../sessions/actions/session-actions.test.ts | 40 ++++++++++++++++++- .../sessions/actions/session-actions.ts | 33 ++++++++++++++- 3 files changed, 73 insertions(+), 2 deletions(-) diff --git a/apps/web/src/features/sessions/actions/SessionActionsDialog.tsx b/apps/web/src/features/sessions/actions/SessionActionsDialog.tsx index 734db26a6..e0f746b9c 100644 --- a/apps/web/src/features/sessions/actions/SessionActionsDialog.tsx +++ b/apps/web/src/features/sessions/actions/SessionActionsDialog.tsx @@ -293,7 +293,9 @@ export function SessionActionsDialog({ uncertainDeleteSessionRef.current = current.id; setDeleteRetryBlocked(true); } + // Offer cancellation only for work it can stop; pending input cannot be cancelled. if (error instanceof SessionActionError && error.kind === "session_busy") setDeleteBusy(true); + if (error instanceof SessionActionError && error.kind === "session_input_pending") setDeleteBusy(false); setActionError(errorMessage(error)); } } finally { diff --git a/apps/web/src/features/sessions/actions/session-actions.test.ts b/apps/web/src/features/sessions/actions/session-actions.test.ts index ac7574f52..34edaee80 100644 --- a/apps/web/src/features/sessions/actions/session-actions.test.ts +++ b/apps/web/src/features/sessions/actions/session-actions.test.ts @@ -297,7 +297,8 @@ describe("Session deletion", () => { new AgentCoreError(busyMessage, 409, "conflict_error", null, "conflict_error"), ); const cancelTurn = vi.fn(); - const core = { deleteSession, cancelTurn } as unknown as AgentCore; + const retrieveSession = vi.fn().mockResolvedValue({ ...session("session-1"), status: "in_progress" }); + const core = { deleteSession, cancelTurn, retrieveSession } as unknown as AgentCore; const error = await requestSessionDelete(core, "session-1").catch((value: unknown) => value); expect(error).toBeInstanceOf(SessionActionError); expect((error as SessionActionError).message).toContain("only when it is idle or failed without required actions"); @@ -307,6 +308,43 @@ describe("Session deletion", () => { expect(cancelTurn).not.toHaveBeenCalled(); }); + it("does not offer cancellation when only input waiting for its Environment blocks deletion", async () => { + const deleteSession = vi.fn().mockRejectedValue( + new AgentCoreError(busyMessage, 409, "conflict_error", null, "conflict_error"), + ); + const cancelTurn = vi.fn(); + const awaitingConnection = { + ...session("session-1"), + status: "requires_action" as const, + required_actions: [{ type: "environment_connection" as const, environment_id: "environment-1" }], + }; + const functionAction = { + ...session("session-1"), + status: "requires_action" as const, + required_actions: [{ type: "function_call" as const, call_id: "call-1", turn_id: "turn-1", name: "lookup", arguments: {} }], + }; + const retrieveSession = vi.fn() + .mockResolvedValueOnce(session("session-1")) + .mockResolvedValueOnce({ ...session("session-1"), status: "failed", error: "The environment is no longer available for this input." }) + .mockResolvedValueOnce(awaitingConnection) + .mockResolvedValueOnce(functionAction) + .mockRejectedValueOnce(new AgentCoreError("unavailable", 503)); + const core = { deleteSession, cancelTurn, retrieveSession } as unknown as AgentCore; + + for (const kind of ["session_input_pending", "session_input_pending", "session_input_pending", "session_busy", "session_busy"]) { + const failure = await requestSessionDelete(core, "session-1").catch((value: unknown) => value); + expect(failure).toMatchObject({ kind }); + if (kind === "session_input_pending") { + expect((failure as Error).message).toContain("cannot be cancelled"); + expect((failure as Error).message).toContain("starts, expires or fails"); + expect((failure as Error).message).not.toContain("Cancel work and delete"); + } + } + expect(deleteSession).toHaveBeenCalledTimes(5); + expect(retrieveSession).toHaveBeenCalledTimes(5); + expect(cancelTurn).not.toHaveBeenCalled(); + }); + it("sends one explicit cancellation before delete and keeps the Session on failure", async () => { const cancelTurn = vi.fn().mockResolvedValue(undefined); const deleteSession = vi.fn(); diff --git a/apps/web/src/features/sessions/actions/session-actions.ts b/apps/web/src/features/sessions/actions/session-actions.ts index 6b6701b57..5f5fbe2e8 100644 --- a/apps/web/src/features/sessions/actions/session-actions.ts +++ b/apps/web/src/features/sessions/actions/session-actions.ts @@ -20,6 +20,7 @@ export type SessionActionFailureKind = | "not_found" | "lifecycle_conflict" | "session_busy" + | "session_input_pending" | "core_unavailable" | "metadata_conflict" | "request_failed" @@ -398,12 +399,42 @@ export async function requestSessionUpdate( } } +/** + * A Session that reads idle or failed, or only awaits its Environment + * connection, yet cannot be deleted is holding input that has not started. + * Core rejects cancellation while that input is pending. + */ +function onlyInputPending(session: AgentSession): boolean { + if (session.status === "idle" || session.status === "failed") return true; + return session.status === "requires_action" && + session.required_actions.length > 0 && + session.required_actions.every((action) => action.type === "environment_connection"); +} + +// Reads the Session once after a busy conflict so the dialog only offers +// cancellation when there is work that cancellation can stop. +async function classifyBusyDelete(core: AgentCore, sessionId: string, busy: SessionActionError): Promise { + let latest: AgentSession; + try { + latest = await retrieveCanonicalSession(core, sessionId); + } catch { + return busy; + } + if (!onlyInputPending(latest)) return busy; + return new SessionActionError( + "Agent Core deletes a Session only when it is idle or failed without required actions. This Session has input waiting to start in its Environment, which cannot be cancelled, so nothing was changed. Delete it after that input starts, expires or fails; once it starts, cancel its work first.", + "session_input_pending", + { cause: busy.cause }, + ); +} + export async function requestSessionDelete(core: AgentCore, sessionId: string): Promise { let deleted: SessionDeleted; try { deleted = await core.deleteSession(sessionId); } catch (error) { - throw normalizeSessionActionError(error, "delete", "write"); + const failure = normalizeSessionActionError(error, "delete", "write"); + throw failure.kind === "session_busy" ? await classifyBusyDelete(core, sessionId, failure) : failure; } if ( deleted?.id !== sessionId || From eb272fabe0d1528989eb5795f9881777f9d57490 Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 13:11:13 +0000 Subject: [PATCH 6/8] Report a connection change between cancel and delete accurately When the Core connection changes after the cancellation but before any deletion, say that the cancellation was sent and no deletion was attempted, instead of blaming an earlier-connection deletion confirmation. --- apps/web/src/App.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/apps/web/src/App.tsx b/apps/web/src/App.tsx index ac7f85405..6280f1e15 100644 --- a/apps/web/src/App.tsx +++ b/apps/web/src/App.tsx @@ -2025,7 +2025,12 @@ export function App() { const isCurrent = () => generation === connectionGenerationRef.current; await requestSessionCancelBeforeDelete(core, sessionId, createIdempotencyKey()); const settled = await waitForSessionIdle(core, sessionId, { isCurrent }); - if (settled === "stale" || !isCurrent()) return false; + if (settled === "stale" || !isCurrent()) { + throw new SessionActionError( + "The cancellation was sent, but the Core connection changed before deletion, so no deletion was attempted. The current Core view was kept.", + "request_failed", + ); + } if (settled === "missing") { return removeSessionFromWorkspace(sessionId, "Session is absent from Agent Core after cancellation."); } From ca16d90aa9964c24d2d90e79f8f7568cf626685e Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 13:12:11 +0000 Subject: [PATCH 7/8] Document the deletion rule's root-Turn scope and Web pending-input case Deletion checks only root Turns and input reservations; subagent child Turns and pending Environment file writes do not block it, as before, and their official behavior is unobserved. State this in the handler annotation, OpenAPI, contributor rules and contract docs, and record it as a follow-up. Describe the Web pending-input explanation, the 30-second bound checked between reads, and re-wrap the Core Web paragraph. --- CONTRIBUTING.md | 15 ++++++++----- contracts/agents-api/README.md | 5 +++-- .../official-semantics-alignment.md | 22 ++++++++++++++----- contracts/agents-api/openapi.yaml | 14 +++++++----- contracts/agents-api/operation-evidence.md | 2 +- docs/web/protocol-coverage.md | 20 +++++++++++------ services/agents-api/README.md | 6 +++-- .../internal/api/session_deletion.go | 2 +- 8 files changed, 56 insertions(+), 30 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c44697f65..52f20cd26 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -34,9 +34,12 @@ the Session creation catalog and uses the existing public client operations. Pat only edited fields, confirm deletion, and never automatically retry an uncertain write. When Core refuses to delete a busy Session, offer an explicit Cancel work and delete action that cancels once, reads until the Session is idle within a -bounded wait and deletes once; never cancel without that confirmation. A Core connection change must discard the previous connection's forms, -pending results and notices. Saving a Template must not allocate a Runtime, call a -model or imply execution readiness. Keep unsupported advanced profiles explicit. +bounded wait and deletes once; never cancel without that confirmation. When only +input waiting for its Environment blocks deletion, explain that it must start, +expire or fail instead, because Core rejects its cancellation. A Core connection +change must discard the previous connection's forms, pending results and notices. +Saving a Template must not allocate a Runtime, call a model or imply execution +readiness. Keep unsupported advanced profiles explicit. For subsequent alignment and milestone closure batches, the main thread coordinates design, shared interface agreements, file ownership, integration and merge. First @@ -1647,8 +1650,10 @@ replaced; do not carry obsolete compatibility code forward to satisfy this secti Store aliases preserve existing callers during this transition. - Session deletion uses a durable `sessions.deleted_at` marker. Public deletion accepts only a durably idle or failed Session without required actions: no - queued, in-progress or waiting Turn and no pending input reservation, the same - settlement rule as the creation stream. Take that decision and commit the marker + queued, in-progress or waiting root Turn and no pending input reservation, the + same settlement rule as the creation stream. Subagent child Turns and pending + Environment file writes are not checked, as before this rule; their official + behavior is unobserved. Take that decision and commit the marker under the tenant Session lock that orders Turn and input admission, so either admission commits first and deletion conflicts, or admission observes the deletion. A busy Session returns 409 `conflict_error` with the observed official diff --git a/contracts/agents-api/README.md b/contracts/agents-api/README.md index 5e8bd559d..9adb0a19e 100644 --- a/contracts/agents-api/README.md +++ b/contracts/agents-api/README.md @@ -306,8 +306,9 @@ upgrade the protocol. - `DELETE /agents/sessions/{session_id}` returns the canonical `id`, `object=agent.session.deleted` and `deleted=true` after durable public removal of a durably idle or failed Session without required actions or pending input. - A queued, running or waiting Turn or pending input returns 409 `conflict_error` - without any change; callers cancel first and delete once idle. Session/Turn/Items + A queued, running or waiting root Turn or pending input returns 409 + `conflict_error` without any change; callers cancel first and delete once idle. + Subagent child Turns and pending Environment file writes do not block deletion. Session/Turn/Items reads, live streams, metadata updates and new input exclude the resource. Existing streams close on observing removal without an invented deletion event. Creation keys remain reserved (local 409); the owner's repeated deletion returns diff --git a/contracts/agents-api/official-semantics-alignment.md b/contracts/agents-api/official-semantics-alignment.md index 548a5c38f..38da89f42 100644 --- a/contracts/agents-api/official-semantics-alignment.md +++ b/contracts/agents-api/official-semantics-alignment.md @@ -289,14 +289,14 @@ first returned 409. | --- | --- | --- | | D1 | DELETE of the caller's own Session that is already publicly deleted (SES-29) | 200 `{id, object: "agent.session.deleted", deleted: true}`, identical to the first confirmation, with no database write. GET, update, events, Turns and Items stay 404. | | D2 | DELETE of a never-existing, malformed or foreign Session, including a foreign deleted one | Unchanged: the byte-identical 404 `not_found_error` of a missing Session. | -| D3 | DELETE while a Turn is queued, in progress (including a requested cancellation) or waiting on required actions or function results, or while an input reservation is pending: a queued later input, self-hosted input awaiting a connection, or hosted initial input while provisioning (SES-30) | 409 with type and code `conflict_error`, param null and message "session must be durably idle or failed without required actions before deletion". Nothing changes: no cancellation, marker, event, Artifact removal or Runtime cleanup. | +| D3 | DELETE while a root Turn is queued, in progress (including a requested cancellation) or waiting on required actions or function results, or while an input reservation is pending: a queued later input, self-hosted input awaiting a connection, or hosted initial input while provisioning (SES-30). Subagent child Turns and pending Environment file writes are not checked (see follow-ups) | 409 with type and code `conflict_error`, param null and message "session must be durably idle or failed without required actions before deletion". Nothing changes: no cancellation, marker, event, Artifact removal or Runtime cleanup. | | D4 | DELETE of an idle Session, including an idle hosted Session still provisioning without input, and of a failed Session without required actions, including expired initial input | 200 with the existing public deletion and managed Runtime cleanup. | | D5 | Callers that need to delete running work | Cancel first with `agent.session.input.cancel`, wait until the Session is idle, then delete. The Core Web offers this as an explicit action after a 409. | Decisions: - The rule is the one the creation stream already uses to settle: the Session is - idle or failed, no Turn is queued, running or waiting, and the latest input + idle or failed, no root Turn is queued, running or waiting, and the latest input reservation is not pending. Deletion reuses the Store's active-Turn query and reservation state, so a pending reservation blocks deletion even while the public status projects idle. @@ -319,10 +319,20 @@ Decisions: markers can remain in upgraded databases; hidden-work settlement, restart reconciliation and Runtime cleanup keep handling them unchanged. - The Core Web keeps the plain delete action. When Core returns the busy 409, the - dialog explains it and replaces the action with Cancel work and delete, which - sends one cancellation, reads the Session until it is idle or failed without - required actions (bounded to 30 seconds) and sends one deletion. A rejected or - uncertain cancellation, a timeout or another 409 stops without retrying. + dialog reads the Session once. If a Turn is still busy it replaces the action + with Cancel work and delete, which sends one cancellation, reads the Session + until it is idle or failed without required actions (a 30-second bound checked + between reads) and sends one deletion. A rejected or uncertain cancellation, a + timeout, a connection change or another 409 stops without retrying. If the + Session reads idle or failed, or only awaits its Environment connection, only + pending input blocks deletion; Core rejects its cancellation, so the dialog + explains that the input must start, expire or fail first and offers no + cancellation. + +Follow-up: deletion checks only root Turns and input reservations. A subagent child +Turn that is still running and a pending Environment file write do not block it, +which matches the permissive behavior before this batch. The official behavior for +both is unobserved; decide whether they should return 409 once it is sampled. Unchanged: physical retention and purge (SESSION-CLEANUP-001 remainder), 404 for reads of deleted Sessions, Artifact retention rules after deletion, managed Runtime diff --git a/contracts/agents-api/openapi.yaml b/contracts/agents-api/openapi.yaml index 336c8f545..d3fda78ce 100644 --- a/contracts/agents-api/openapi.yaml +++ b/contracts/agents-api/openapi.yaml @@ -3812,12 +3812,14 @@ paths: /agents/sessions/{session_id}: delete: description: Removes a durably idle or failed Session and its history from the - public API. A Session with a queued, in-progress or waiting Turn, required - actions or pending input returns 409 conflict_error and is left unchanged; - cancel it and wait until it is idle before deleting. Repeating the deletion - of the caller's own deleted Session returns the same confirmation; missing - and foreign Sessions return 404. Internal records and native history are retained - pending separate physical cleanup; overlapping stream timing remains unverified. + public API. A Session whose root Turn is queued, in progress or waiting (including + required actions) or whose input reservation is pending returns 409 conflict_error + and is left unchanged; cancel it and wait until it is idle before deleting. + Subagent child Turns and pending Environment file writes are not checked and + do not block deletion. Repeating the deletion of the caller's own deleted + Session returns the same confirmation; missing and foreign Sessions return + 404. Internal records and native history are retained pending separate physical + cleanup; overlapping stream timing remains unverified. parameters: - description: agents=v1 in: header diff --git a/contracts/agents-api/operation-evidence.md b/contracts/agents-api/operation-evidence.md index cf2744c44..5f51fcb19 100644 --- a/contracts/agents-api/operation-evidence.md +++ b/contracts/agents-api/operation-evidence.md @@ -58,7 +58,7 @@ Paths in the appendix include `/v1`. SDK names here omit `client.`. `P` means pa | 7 | beta.agents.sessions.retrieve | P: persisted state, required actions, usage; malformed ID equals missing | S `retrieve-1.json`, `session-after-1.json`; H recovered state; X SES-28 | C Live history; T pending actions; H Core acceptance recorded | Complete statuses/actions/lifecycle timing; Claude/MiniMax public usage remains null | | 8 | beta.agents.sessions.update | P: metadata-only replacement/clear; metadata errors use official code and `metadata`/`metadata.` param | S `metadata-replace/null/empty/omit/invalid-value.json`; `update-agent.json` uses newer unpinned field | N Live completed Session metadata rejection/clear/isolation; recorded active controlled metadata coverage | Session admission batch changes empty update to observed official 400; Session agent update belongs to baseline upgrade, not fixed-pin operation gap | | 9 | beta.agents.sessions.list | P: Agent filter, full envelope, cursor paging; unknown keys ignored, limit 0/above 100 clamp | S `list-filter.json`, `list-empty-after.json`, `list-owned-cross-filter-cursor.json`, limit/order/unknown-query samples; L SES-10/11/15/16/17 | C Live order/cursors/empty/tenant checks; L DB tenant A/B | Core page capacity 100; official cap unknown. Eventual visibility sample is not a required delay | -| 10 | beta.agents.sessions.delete | P: deletion only of a durably idle or failed Session without required actions or pending input; otherwise 409 `conflict_error` with no change; owner repeat returns the same 200; owned managed cleanup, user compute retained | S cleanup files 200/deleted; retry-session active cleanup initially 409; Z SES-29 `q5-delete-repeat` 200, SES-30 `q5b-delete-while-in-progress` 409, `q5-delete-never-existed` 404, `q5-delete-while-running` 200 right after an `events.create` 202 | D recorded real cleanup; C cleanup separately recorded; Z DB matrix D1–D4 with no-write digest, admission lock race and pinned-SDK script | Core returns 409 right after an `events.create` 202 because it admits Turns synchronously (official 200); input awaiting its Environment cannot be cancelled publicly and is unobserved officially; physical purge/retention may end repeat idempotency; caller compute ownership preserved | +| 10 | beta.agents.sessions.delete | P: deletion only of a durably idle or failed Session without required actions or pending input; a busy root Turn or pending reservation gives 409 `conflict_error` with no change (subagent child Turns and pending Environment file writes are not checked); owner repeat returns the same 200; owned managed cleanup, user compute retained | S cleanup files 200/deleted; retry-session active cleanup initially 409; Z SES-29 `q5-delete-repeat` 200, SES-30 `q5b-delete-while-in-progress` 409, `q5-delete-never-existed` 404, `q5-delete-while-running` 200 right after an `events.create` 202 | D recorded real cleanup; C cleanup separately recorded; Z DB matrix D1–D4 with no-write digest, admission lock race and pinned-SDK script | Core returns 409 right after an `events.create` 202 because it admits Turns synchronously (official 200); input awaiting its Environment cannot be cancelled publicly and is unobserved officially; physical purge/retention may end repeat idempotency; caller compute ownership preserved | | 11 | beta.agents.sessions.events.create | P: 202/empty body, empty-array authenticated no-op, text/cancel/function admission | S `second-turn-create.json`, `events-empty/null.json`; H second-input | C Live real continuation/no-op; T qualified message/result/cancel workflows | Mixed prepared-environment batches, native receipt vs durable acceptance, cancel-before-result-publication timing; unqualified content/tools | | 12 | beta.agents.sessions.events.stream | P: live-only SSE, typed persisted projections; terminal Turn events carry top-level Turn usage (null when unknown); new Turns start `turn.created`, user `item.added`, `session.in_progress`, `turn.in_progress` | H create/reconnect frames; no historical frames in sampled idle interval; J GET streams never ended, terminal `usage` present | C Live; H recorded three-harness disconnect/recovery; T pending actions; J DB order/usage | Full SSE/Item variants/order (EVT-05..10 deferred); child deltas settle late; no replay guarantee or observer-disconnect proof for every state | | 13 | beta.agents.sessions.turns.retrieve | P: persisted root/child Turn identity; malformed ID equals missing | H/S contain Turn list payloads; no isolated positive retrieve raw request identified in this set; X SES-28 official `turn_` 404 | Recorded H/B scoped Turn recovery; C history uses list | Distinguish list-shape evidence from retrieve wire qualification; full lifecycle/usage | diff --git a/docs/web/protocol-coverage.md b/docs/web/protocol-coverage.md index 3d070c0b2..f32daf4fe 100644 --- a/docs/web/protocol-coverage.md +++ b/docs/web/protocol-coverage.md @@ -514,13 +514,19 @@ upstream. deleted position or the previous item at the end. Deleting an inactive Session does not change the selected ID, stream epoch, composer, or current conversation state. - Core deletes only a durably idle or failed Session without required actions or - pending input; otherwise it returns 409 `conflict_error` and changes nothing. The - dialog then explains the conflict and replaces its action with Cancel work and - delete. Only that explicit choice sends one `agent.session.input.cancel`, reads the - Session until it is idle or failed without required actions (at most 30 seconds) - and sends one deletion. A rejected or uncertain cancellation, a timeout, a - connection change or another 409 stops without a retry; other 409 responses keep - the generic lifecycle-conflict message. + pending input: a queued, running or waiting root Turn or a pending input + reservation returns 409 `conflict_error` and changes nothing. Subagent child + Turns and pending Environment file writes are not checked. After that 409 the + dialog reads the Session once. If a Turn is still busy, it replaces its action + with Cancel work and delete. Only that explicit choice sends one + `agent.session.input.cancel`, reads the Session until it is idle or failed + without required actions and sends one deletion; a 30-second bound is checked + between reads. A rejected or uncertain cancellation, a timeout, a connection + change or another 409 stops without a retry. If the Session reads idle or + failed, or only awaits its Environment connection, only pending input blocks + deletion and Core rejects its cancellation, so the dialog explains that the + input must start, expire or fail first and offers no cancellation. Other 409 + responses keep the generic lifecycle-conflict message. - Parsar deletion is a public server lifecycle operation. It hides the durable public Session/Items/Turns and closes its stream. It does not prove immediate native executor quiescence, physical SQL/native-history erasure, or deletion of executor Workspace files. diff --git a/services/agents-api/README.md b/services/agents-api/README.md index 869a935f8..56d2650d6 100644 --- a/services/agents-api/README.md +++ b/services/agents-api/README.md @@ -237,8 +237,10 @@ Session updates require the metadata field; null/empty clears it and an object replaces supplied pairs. An empty update body rejects before resource lookup. Delete with `client.beta.agents.sessions.delete(session.id)`. Only a durably idle -or failed Session without required actions or pending input can be deleted; any -other Session returns 409 `conflict_error` and is left unchanged. Cancel its work +or failed Session without required actions or pending input can be deleted; a +queued, running or waiting root Turn or a pending input reservation returns 409 +`conflict_error` and leaves the Session unchanged. Subagent child Turns and +pending Environment file writes do not block deletion. Cancel its work with an `agent.session.input.cancel` event, wait until it is idle, then delete it. Confirmation means public removal: Session/history reads and new input become unavailable, and existing streams close on observing removal. Creation keys stay diff --git a/services/agents-api/internal/api/session_deletion.go b/services/agents-api/internal/api/session_deletion.go index f896b2cb3..48071f683 100644 --- a/services/agents-api/internal/api/session_deletion.go +++ b/services/agents-api/internal/api/session_deletion.go @@ -11,7 +11,7 @@ import ( ) // @Summary Delete an execution Session -// @Description Removes a durably idle or failed Session and its history from the public API. A Session with a queued, in-progress or waiting Turn, required actions or pending input returns 409 conflict_error and is left unchanged; cancel it and wait until it is idle before deleting. Repeating the deletion of the caller's own deleted Session returns the same confirmation; missing and foreign Sessions return 404. Internal records and native history are retained pending separate physical cleanup; overlapping stream timing remains unverified. +// @Description Removes a durably idle or failed Session and its history from the public API. A Session whose root Turn is queued, in progress or waiting (including required actions) or whose input reservation is pending returns 409 conflict_error and is left unchanged; cancel it and wait until it is idle before deleting. Subagent child Turns and pending Environment file writes are not checked and do not block deletion. Repeating the deletion of the caller's own deleted Session returns the same confirmation; missing and foreign Sessions return 404. Internal records and native history are retained pending separate physical cleanup; overlapping stream timing remains unverified. // @Tags Sessions // @Produce json // @Security BearerAuth From 75a8a3f3b44f691d03a1a25115ff480ec0bc798d Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 14:02:41 +0000 Subject: [PATCH 8/8] Test and document placement retention for undeletable hosted input A provisioning hosted Session with reserved initial input on a configured sandbox node now conflicts on deletion, so its placement keeps counting toward node capacity until the input is admitted or expires. A real store test checks the unreleased placement and unchanged node counts on the 409, release on the allowed deletion after expiry, unchanged timestamps on a repeat and 404 for another tenant. Record the capacity change in the contributor rules and the deletion alignment section. --- CONTRIBUTING.md | 4 ++ .../official-semantics-alignment.md | 5 ++ .../internal/store/session_deletion_test.go | 70 +++++++++++++++++++ 3 files changed, 79 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 52f20cd26..19f81be3e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1661,6 +1661,10 @@ replaced; do not carry obsolete compatibility code forward to satisfy this secti cancel first (`agent.session.input.cancel`), wait until the Session is idle and delete it. Core admits a Turn synchronously, so it also conflicts right after an `events.create` 202, where the official service was observed to return 200. + Deleting a provisioning hosted Session with reserved input used to release its + sandbox node placement at once; it now conflicts, and the placement counts + toward node capacity until the input is admitted or its five-minute deadline + expires. A later allowed deletion releases an unallocated placement. The owner's repeated deletion returns the same 200 confirmation without writing; foreign, missing and malformed identifiers keep the byte-identical 404. Public reads, metadata changes, event streams and input admission exclude deleted diff --git a/contracts/agents-api/official-semantics-alignment.md b/contracts/agents-api/official-semantics-alignment.md index 38da89f42..2cbbb3627 100644 --- a/contracts/agents-api/official-semantics-alignment.md +++ b/contracts/agents-api/official-semantics-alignment.md @@ -315,6 +315,11 @@ Decisions: cannot be cancelled publicly (the pending reservation rejects new batches), so it stays undeletable until the input starts, its five-minute deadline expires or its Environment fails. The official behavior of that window is unobserved. +- Capacity change: previously, deleting a provisioning hosted Session with + reserved input released its sandbox node placement immediately. Now the + deletion returns 409, and the placement keeps counting toward the node's + retained and reserved capacity until the input is admitted or its five-minute + deadline expires. A later allowed deletion releases an unallocated placement. - Earlier releases deleted busy Sessions after requesting cancellation. Their markers can remain in upgraded databases; hidden-work settlement, restart reconciliation and Runtime cleanup keep handling them unchanged. diff --git a/services/agents-api/internal/store/session_deletion_test.go b/services/agents-api/internal/store/session_deletion_test.go index 84ca0f338..1f0162cf4 100644 --- a/services/agents-api/internal/store/session_deletion_test.go +++ b/services/agents-api/internal/store/session_deletion_test.go @@ -362,3 +362,73 @@ func TestSessionDeletionRacesAdmissionUnderSessionLock(t *testing.T) { }) } } + +// A provisioning hosted Session with reserved initial input keeps its node +// placement, and so its capacity, until the input settles. Earlier releases +// released the placement immediately; now deletion conflicts until the input is +// admitted or expires, and the later allowed deletion releases it once. +func TestSessionDeletionKeepsProvisioningInputPlacementUntilSettled(t *testing.T) { + s, w, d := managerFixture(t, 2, 4) + ctx := t.Context() + tenant := uuid.NewString() + input := managerSessionInput("reserved-input", d.LocalNodeID) + input.InitialInputs = []Input{messageInput("reserved")} + session, err := s.CreateSession(ctx, tenant, input) + if err != nil { + t.Fatal(err) + } + type state struct { + deleted, released pgtype.Timestamptz + retained, reserved int64 + } + read := func() state { + t.Helper() + var current state + if err := s.pool.QueryRow(ctx, `SELECT s.deleted_at, p.released_at FROM sessions s + JOIN environments e ON e.session_id=s.id JOIN runtime_placements p ON p.environment_id=e.id + WHERE s.id=$1 AND p.node_id=$2`, session.ID, d.LocalNodeID).Scan(¤t.deleted, ¤t.released); err != nil { + t.Fatal("missing placement", err) + } + nodes, err := s.ListRuntimeNodes(ctx) + if err != nil || len(nodes) != 1 { + t.Fatal(nodes, err) + } + current.retained, current.reserved = nodes[0].Retained, nodes[0].Reserved + return current + } + before := read() + if before.deleted.Valid || before.released.Valid || before.retained != 1 || before.reserved != 1 { + t.Fatal("unexpected reserved placement", before) + } + if err := s.DeleteSession(ctx, uuid.NewString(), session.ID); !errors.Is(err, ErrNotFound) { + t.Fatal("foreign deletion", err) + } + if err := s.DeleteSession(ctx, tenant, session.ID); !errors.Is(err, ErrSessionNotIdle) { + t.Fatal("provisioning input deleted", err) + } + if after := read(); after != before { + t.Fatal("rejected deletion released capacity", before, after) + } + if _, err := s.pool.Exec(ctx, "UPDATE environment_input_reservations SET deadline=clock_timestamp()-interval '1 second' WHERE session_id=$1", session.ID); err != nil { + t.Fatal(err) + } + if count, err := w.ExpireEnvironmentInputs(ctx); err != nil || count != 1 { + t.Fatal("initial input did not expire", count, err) + } + if err := s.DeleteSession(ctx, tenant, session.ID); err != nil { + t.Fatal(err) + } + deleted := read() + if !deleted.deleted.Valid || !deleted.released.Valid || deleted.retained != 0 || deleted.reserved != 0 { + t.Fatal("allowed deletion kept the placement", deleted) + } + if err := s.DeleteSession(ctx, tenant, session.ID); err != nil { + t.Fatal("repeated deletion", err) + } + if again := read(); again != deleted { + t.Fatal("repeated deletion changed timestamps", deleted, again) + } + if err := s.DeleteSession(ctx, uuid.NewString(), session.ID); !errors.Is(err, ErrNotFound) { + t.Fatal("foreign deletion of a deleted Session", err) + } +}