fix: reject session create on a soft-deleted id

[QuentinBisson]

Fixes #2279

Summary

UpsertSession had no deleted_at clause, so ON CONFLICT (id, user_id) DO UPDATE matched soft-deleted rows. Creating a session with a deleted id overwrote the tombstone's name, agent_id and source while deleted_at stayed set. Every read filters deleted_at IS NULL, so the write was invisible, including to the create path's own reload, which then returned an internal error. The caller was told the request failed after a write had happened, and the damaged row is the one an audit reads.

• add WHERE session.deleted_at IS NULL to the DO UPDATE and make the query :one, so a rejected write returns no rows
• map that to database.ErrSessionIDRetired, which Service.Create returns as AlreadyExists and Service.Update as NotFound
• UpsertTask already guards the same hazard this way, so the two upserts now match

The guard lives in the statement rather than a pre-check in the service for two reasons: it is atomic (a delete that commits while the upsert waits on the row lock is still rejected, where a pre-read leaves a window), and it applies to all three StoreSession callers instead of one.

Ids are not burned permanently. DeleteExpiredSessionsBatch hard-deletes tombstones past the retention window, after which the id inserts normally again.

Behaviour per id conflict

Existing row	Result
same user, live	200, upsert (unchanged; the UI's chat rename in ChatInterface.tsx is a create against a live id)
same user, retired	AlreadyExists, nothing written (this PR)
other user, live	unchanged, the conflict target is (id, user_id)
other user, retired	200, insert, a tombstone is not a live row

One path outside the create RPC changes. ensureSessionRow in the sandbox A2A transport previously performed the invisible write against a retired id and let the request proceed; it now writes nothing and RoundTrip fails the request. Continuing would create an actor for an id that can never hold a visible row, so no read path and no Service.Delete could reach it. Other store failures in that best-effort branch still log and proceed.

Testing

New Postgres-backed tests:

• recreating a deleted session returns ErrSessionIDRetired, the tombstone's created_at, deleted_at and name are unchanged, and the same id under a different user_id still inserts
• the direct A2A path and RoundTrip both reject a retired id
• the service maps the sentinel to AlreadyExists on create and NotFound on update, and the generated gRPC client sees codes.AlreadyExists over the wire

TestStoreSessionIdempotence is unchanged and still passes, including its live re-upsert of name.

[Copilot]

Pull request overview

Fixes a correctness bug in the Postgres session upsert path where ON CONFLICT (id, user_id) DO UPDATE could match a soft-deleted (“tombstoned”) session row, causing an invisible write and a subsequent 500 on reload. The change makes soft-deleted session IDs reject writes atomically at the SQL layer and maps that condition through the service and transports to stable, user-meaningful error codes.

Changes:

• Harden UpsertSession to not update soft-deleted rows (WHERE session.deleted_at IS NULL) and return no rows when rejected (mapped to ErrSessionIDRetired).
• Map ErrSessionIDRetired to AlreadyExists on create and NotFound on update, and ensure the sandbox A2A transport fails fast for retired IDs.
• Add Postgres-backed and transport-level tests covering the retired-ID behavior and error-code mapping.

Reviewed changes

Copilot reviewed 9 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
go/core/internal/service/session/service.go	Maps ErrSessionIDRetired to AlreadyExists (create) and NotFound (update).
go/core/internal/service/session/service_test.go	Adds unit test verifying service error-code mapping for retired session IDs.
go/core/internal/grpcserver/session_task_test.go	Updates generated-client test store to simulate “retired ID” behavior and asserts codes.AlreadyExists.
go/core/internal/database/queries/sessions.sql	Updates UpsertSession to skip updates of soft-deleted rows and return id (:one).
go/core/internal/database/gen/sessions.sql.go	Regenerates sqlc output for the new UpsertSession return signature and SQL.
go/core/internal/database/gen/querier.go	Updates the Querier interface for UpsertSession(ctx, ...) (string, error).
go/core/internal/database/client_postgres.go	Maps pgx.ErrNoRows from UpsertSession to dbpkg.ErrSessionIDRetired.
go/core/internal/database/client_test.go	Adds Postgres-backed test asserting retired IDs don’t mutate tombstones and don’t write invisibly.
go/core/internal/a2a/substrate_sandbox_transport.go	Treats retired session IDs as a hard failure in RoundTrip (no best-effort fallback).
go/core/internal/a2a/substrate_sandbox_transport_test.go	Adds tests ensuring both ensureSessionRow and RoundTrip reject retired IDs.
go/api/database/client.go	Introduces the shared sentinel ErrSessionIDRetired.

Files not reviewed (2)

• go/core/internal/database/gen/querier.go: Generated file
• go/core/internal/database/gen/sessions.sql.go: Generated file

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[QuentinBisson]

Reworded: the doc comment now says the id stays unusable only while the tombstone exists, and inserts normally again once retention hard-deletes it. 5135a1f