refactor(agents): rename Chat Session to Conversation, drop the legacy model

[ExtraToast]

Summary

Draft: the contract gate fails by design. Read Validation before merging.

"Session" stops meaning two things. Three commits, deliberately not interleaved — the target name was occupied by the model being deleted, so a single-pass rename would have collided with live files:

1. Drops the legacy Conversation/Message model — domain classes, ports, the codegen-backed jOOQ repositories, both controllers, DTOs, command handlers, query services, tests, and the conversation and message tables.
2. Renames the live model — ChatSession→Conversation, ChatMessage→ConversationMessage, with chat_sessions→conversations and chat_session_messages→conversation_messages by ALTER TABLE ... RENAME, preserving every row. /api/v1/conversations becomes canonical; /api/v1/chat-sessions stays as a deprecated alias on identical shapes, so agents-ui keeps working untouched. Enum constant names are unchanged, since they are persisted by .name.
3. Fixes an authorization gap found by a background review of commit 2 and specced as Conversations are readable and writable by any caller who knows the id #80.

The canonical detail endpoint gains a real ConversationDetailResponse; the old one returned an untyped Map<String, Any>, giving generated-client callers no type at all. The alias keeps the untyped shape.

On #80: GET /{id}, POST /{id}/messages and the stream endpoint took no caller identity and never checked ownership — anyone with a Conversation id could read the Transcript, append Turns as any role, or drive generation. One seam: ConversationQueryService.get now takes the requesting user and returns nothing for someone else's Conversation, indistinguishable from an unknown id, so every existing ?: notFound() does the work and a new handler cannot skip the check. Writes resolve ownership before dispatching, so a refused append leaves no trace. 404 not 403, following WorkspaceRunnerEventsController — a 403 confirms the id exists. The alias inherits the same check.

Validation

• :api:test — 616 tests, 614 passed, 0 failed, 2 pre-existing skips
• :api:integrationTest — 120 tests, 118 passed, 0 failed, 2 skipped
• detekt (main, test, integrationTest) + ktlint --warning-mode=fail — clean
• Spec regenerated and committed; force-rerun with --rerun-tasks

Two cross-user integration tests run against real Postgres on both the canonical and alias paths. The write-path unit tests assert commandBus.dispatch and chatAnswerStream.stream are called zero times on refusal, not merely that the status was 404.

The contract job fails. oasdiff breaking with fail-on: WARN reports 15 errors and 2 warnings, every one on /api/v1/conversations* — the legacy replacement — plus 4 new required X-User-Id headers. Zero findings on /api/v1/chat-sessions: the alias removed all avoidable breakage. api-contract-checks exposes no waiver and I have not touched the gate. Full output in the PR comments.

Of those 15 errors, 3 are pre-existing springdoc drift, not behaviour change: create/message/delete are documented 200 while returning 201/204, because springdoc cannot infer status from a dynamically built ResponseEntity. main's /chat-sessions spec already has this; the move just makes oasdiff compare it against the legacy controller's annotated version.

Notes

• The legacy surface has no caller in this workspace. It is published in the client, so an external consumer cannot be ruled out from here.
• Two ArchUnit rules had hard-coded predicates that would have gone empty. One matched startsWith("ChatSession"); the other matched @Service, an annotation unique to the three deleted handlers. Both now match on interfaces. The new test asserting every Conversation handler takes the identity header was proved non-vacuous by mutation — pointing its filter at a non-existent controller fails the isNotEmpty() guard rather than passing with nothing checked.
• Requires a decision: a waiver in api-contract-checks, a one-off merge past red, or keeping the legacy endpoints alive. The third contradicts Rename Chat Session to Conversation and split navigation into Workspaces and Conversations #68's second criterion.

refs #68, refs #80

[ExtraToast]

Security finding — pre-existing, carried through by this rename, not introduced by it

A background review flagged IDOR on ConversationController. Verified, and the picture is broader than the three endpoints it names.

What is missing

GET /{id}, POST /{id}/messages and POST /{id}/messages/stream take no caller identity and perform no ownership check. Anyone who knows a conversation UUID can read the full transcript, append messages as any role including ASSISTANT or SYSTEM, and drive the answer stream.

Not introduced here. main's ChatSessionController has the identical gap on the same three handlers. This PR renames the file and moves the surface to /api/v1/conversations; it does not change the semantics.

The part the review understates

The other three handlers — create, list, archive — take @RequestHeader("X-User-Id"), which looks like authorization but is not: it is client-supplied, so any caller can assert any user id. Worse, agents-ui deliberately strips it (src/lib/runtimeOrigins.ts:192, headers.delete('X-User-Id'), with tests asserting it is absent), because real identity comes from the authenticated principal.

So ownership on this controller is client-asserted throughout. Adding X-User-Id to the three handlers that lack it would look like a fix and change nothing.

Why it is not fixed in this PR

The real fix is to derive the user from the authenticated principal and check that the Conversation belongs to them, on every handler. That is a behaviour change to the auth model, it touches the deprecated alias too, and it would add further breaking findings on top of the ones this PR already has to resolve. Silently changing auth semantics inside a rename is the wrong place for it.

Recommend a separate issue and PR. Note the legacy surface being removed here did carry x-user-id on its messages endpoint — that is one of the two oasdiff warnings above — so the removal is visible in the contract diff rather than silent.

[ExtraToast]

Ownership fix folded in — #80

Branch force-updated to e597566. Two changes since the draft opened:

1. The authorization gap from the review above is now fixed here, per #80, rather than in a separate PR — these three handlers were already being reshaped by the rename, so folding it in avoids a second breaking change to the same endpoints.

One seam: ConversationQueryService.get now takes the requesting user and returns nothing when the Conversation belongs to someone else, indistinguishable from an unknown id. Every caller already maps that to 404, so no handler gained a branch and a future handler cannot skip the check by forgetting it. appendMessage and streamMessage now resolve ownership before dispatching, so a refused write leaves no trace — previously append dispatched first and read back afterwards. The deprecated /api/v1/chat-sessions alias inherits the same check rather than reimplementing it.

Refusals return 404, not 403, following WorkspaceRunnerEventsController — a 403 would confirm the id exists. They are logged at WARN with the Conversation id and requesting user, so probing stays visible while the response body says nothing.

Evidence: 616 unit tests, 120 integration including two full-stack cross-user tests against real Postgres covering both the canonical and alias paths. The write-path tests assert commandBus.dispatch and chatAnswerStream.stream are called zero times on refusal, not merely that the status was 404.

The new ArchUnit test asserting every Conversation handler takes the identity header was proved non-vacuous by mutation: removing the header from one handler failed it by name, and pointing the class filter at a non-existent controller failed the isNotEmpty() guard rather than passing with nothing checked.

2. History corrected. The previous head carried a 0-byte .kotlin/sessions/*.salive compiler-daemon lock file that I committed by accident with a blind git add -A. It is untracked now and .kotlin/ is in .gitignore. Nothing else differed between the two versions.

Spec: 4 new required X-User-Id header parameters, regenerated and committed. This adds to the contract findings already listed above.

Correction to the earlier findings summary: three of the fifteen oasdiff errors — response-success-status-removed on POST create, POST message and DELETE — are a pre-existing springdoc quirk, not a change in behaviour. main's /chat-sessions spec already documents those as 200 while the handlers return 201/204 at runtime, because springdoc cannot infer the status from a dynamically built ResponseEntity. Moving the controller onto /conversations just makes oasdiff compare it against the legacy controller's annotated version for the first time. Runtime behaviour is unchanged, and I have not touched it.

[ExtraToast]

Contract findings cut from 17 to 11 — still red, and the remainder cannot be fixed honestly

b5f5c8a removes every finding that was an artefact of reshaping the canonical surface rather than a real difference. Nothing under .github/ was touched.

Fixed

• 3 × response-success-status-removed — create, appendMessage and archive are annotated with the status they actually return. main's spec claimed 200 where the code returns 201/204, because springdoc cannot infer status from a dynamically built ResponseEntity. The spec is now more truthful; runtime is unchanged.
• api-removed-without-deprecation — GET /{conversationId}/messages restored, served by the renamed model through the same ConversationQueryService.get seam, so it inherits Conversations are readable and writable by any caller who knows the id #80's ownership check rather than being a public read hole.
• 6 × response-required-property-removed — ConversationDetailResponse flattened back to the legacy shape. The nesting was my design preference, not a requirement.
• new-required-request-property body/role and request-property-removed content — the canonical path accepts the legacy content shape and defaults role to USER. ConversationMessageResponse also carries content alongside body: once the status fix let oasdiff compare 201 bodies for the first time, main's content field became visible and would otherwise have reopened as a new finding.

Not fixed, deliberately — 11 remaining

• 4 × new-required-request-parameter x-user-id on GET /conversations/{id}, GET /conversations/{id}/messages, GET /chat-sessions/{id} and POST /chat-sessions/{id}/messages. These are Conversations are readable and writable by any caller who knows the id #80. The endpoints required no identity before because they had no ownership check. Removing the finding means removing the check.
• 6 × title became nullable/optional across three endpoints. Conversation.title is String? and blank titles are coerced to null. Declaring it non-null would misrepresent real responses.
• 1 × request-property-max-length-decreased to 120. conversations.title is VARCHAR(120) in V9, carried through V27 untouched. My instruction to relax this to 200 was wrong — it would let a 121–200 character title pass validation and then fail at insert, and the spec would advertise a limit the database does not honour.

So the gate cannot go green for this change without either weakening the ownership fix or publishing a spec that lies. Both are worse than a red check.

Eight of the eleven only became visible once the status-code fixes let oasdiff compare matching response codes — previously masked by a 200-vs-201 mismatch being treated as unrelated. They are pre-existing differences surfacing, not new breakage.

Full suite green: 619 unit (617 passed, 2 pre-existing skips), 120 integration (118 passed, 2 skipped), detekt and ktlint clean. #80's cross-user tests still pass on both paths, now including the restored messages endpoint.