docs(openspec): tighten run isolation contract

openspec/changes/session-run-trace-isolation/decisions.md

@@ -133,3 +133,11 @@ Audit conclusions:
 - Added `DiagnosisRunRepositoryTest`.
 - Verification evidence is recorded in `phase-1-evidence.md`.
 - Historical DB inspection found one orphan `tool_invocation` row without a matching `diagnosis_session`; it remains `run_id = NULL` because no reliable compatibility run can be inferred.
+
+## Document Review Follow-up
+
+- Clarified that Chat creates runs for the effective resolved `sessionId`, including requests where the server generates a session id.
+- Tightened legacy feedback fallback so `fallbackToLatestRun=true` and the actual bound `runId` are response fields, not log-only evidence.
+- Clarified AIOps SSE compatibility: emit a metadata message containing `sessionId` and `runId` before report content while preserving the existing content stream shape.
+- Clarified that run/session ownership is enforced by service-layer validation and indexed lookup in this change; database foreign keys are intentionally deferred to preserve compatibility with historical orphan detail rows and rollback.
+- Clarified that `chat_session.expires_at` is nullable directory metadata / best-effort TTL snapshot, not mandatory persisted conversation history.

openspec/changes/session-run-trace-isolation/design.md

@@ -46,6 +46,7 @@ Constraints from existing work:
 | Feedback fallback | Missing `runId` binds latest run and returns fallback metadata | Reject missing `runId` immediately | Short-term compatibility is needed for old clients; explicit fallback keeps ambiguity observable. |
 | Case library provenance | New automatic cases store `diagnosis_id = run_id` | Add a new case-library column now | Existing column name can carry transitional provenance; docs and query logic must recognize old `session_id` and new `run_id`. |
 | AIOps phase | Implement after Chat but before overall archive | Leave AIOps for a follow-up issue | AIOps is already a trace entry point; leaving it old-model would preserve the same bug on another endpoint. |
+| Ownership integrity | Validate run/session ownership in application services; do not add database foreign keys in this change | Add foreign keys from `diagnosis_run`, `agent_step`, and `tool_invocation` | Existing historical/orphan compatibility data and rollback needs make additive, application-level validation safer for this release. |
 
 ## Data Model
 
@@ -87,7 +88,9 @@ tool_invocation
   ...
 ```
 
-`chat_session.expires_at` is MySQL directory metadata. Redis TTL can expire `SessionContext.messageHistory`; persisted `diagnosis_run`, `agent_step`, and `tool_invocation` remain audit records.
+`chat_session.expires_at` is nullable MySQL directory metadata and may be a best-effort Redis TTL snapshot when known. Redis TTL can expire `SessionContext.messageHistory`; persisted `diagnosis_run`, `agent_step`, and `tool_invocation` remain audit records.
+
+Ownership between `chat_session`, `diagnosis_run`, `agent_step`, and `tool_invocation` is enforced by service-layer validation and indexed lookup in this change. The migration intentionally does not add database foreign keys so historical orphan trace detail rows and rollback paths remain compatible.
 
 ## API / Interface Impact
 
@@ -95,7 +98,7 @@ Interface level: L4.
 
 - Database contract changes: new tables, new columns, backfill, indexes, and later non-null expectations for new writes.
 - `/api/chat` response adds `runId`.
-- `/api/ai_ops` SSE emits the resolved `runId`.
+- `/api/ai_ops` SSE emits a compatible metadata message before report content. The metadata payload includes `sessionId` and `runId`; report content continues to stream through the existing content message shape.
 - Trace API accepts optional `runId`.
 - Feedback request accepts preferred `runId` and returns fallback binding metadata when omitted.
 - New run summary API: `GET /api/chat/session/{sessionId}/runs`.
@@ -136,4 +139,3 @@ Rollback:
 ## Open Questions
 
 None blocking. Long-term tightening of missing feedback `runId` remains a follow-up decision after clients migrate.
-

openspec/changes/session-run-trace-isolation/specs/session-run-trace-isolation/spec.md

@@ -4,10 +4,10 @@
 The system SHALL persist multi-turn conversation metadata in `chat_session` and one execution's auditable state in `diagnosis_run`.
 
 #### Scenario: Valid Chat execution creates session metadata and a run
-- **WHEN** a valid `/api/chat` request enters the Chat execution path with a `sessionId`
+- **WHEN** a valid `/api/chat` request enters the Chat execution path and the service resolves an effective `sessionId`
-- **THEN** the system SHALL ensure a `chat_session` row exists for that `sessionId`
+- **THEN** the system SHALL ensure a `chat_session` row exists for the effective `sessionId`
 - **AND** it SHALL create a new `diagnosis_run` row with a unique `run_id`
-- **AND** the `diagnosis_run.session_id` SHALL equal the request `sessionId`
+- **AND** the `diagnosis_run.session_id` SHALL equal the effective `sessionId`
 
 #### Scenario: Invalid Chat request does not create a run
 - **WHEN** a `/api/chat` request fails parameter validation before execution
@@ -15,7 +15,7 @@ The system SHALL persist multi-turn conversation metadata in `chat_session` and
 
 #### Scenario: Chat session stores metadata only
 - **WHEN** a Chat request completes
-- **THEN** `chat_session` SHALL store metadata such as status, message pair count, created time, last active time, and expiration time
+- **THEN** `chat_session` SHALL store metadata such as status, message pair count, created time, last active time, and optional expiration time
 - **AND** it SHALL NOT store full conversation message history
 
 ### Requirement: Chat responses SHALL expose run identity
@@ -86,8 +86,8 @@ The system SHALL bind new feedback to a diagnosis run rather than an ambiguous m
 #### Scenario: Feedback without runId falls back observably
 - **WHEN** a legacy feedback request includes `sessionId` but omits `runId`
 - **THEN** the system SHALL bind feedback to the latest run for that session
-- **AND** the response or log SHALL include `fallbackToLatestRun=true`
+- **AND** the response SHALL include `fallbackToLatestRun=true`
-- **AND** the response or log SHALL include the actual bound `runId`
+- **AND** the response SHALL include the actual bound `runId`
 
 #### Scenario: Useful feedback creates case from run
 - **WHEN** feedback for a run is `useful`
@@ -104,8 +104,9 @@ The system SHALL create and expose a diagnosis run for every valid `/api/ai_ops`
 
 #### Scenario: AIOps SSE exposes runId
 - **WHEN** `/api/ai_ops` streams response metadata to the caller
-- **THEN** the stream SHALL expose the resolved `sessionId`
+- **THEN** the stream SHALL send a compatible metadata message before report content
-- **AND** it SHALL expose the created `runId`
+- **AND** the metadata payload SHALL expose the resolved `sessionId`
+- **AND** the metadata payload SHALL expose the created `runId`
 
 ### Requirement: Migration SHALL preserve historical trace access
 The system SHALL migrate historical diagnosis data into compatibility runs without deleting the old `diagnosis_session` table.
@@ -144,4 +145,3 @@ The change SHALL verify both runtime behavior and evaluation baseline impact.
 - **WHEN** verification is complete
 - **THEN** the project SHALL run or explicitly evaluate the relevant baseline diff command
 - **AND** any drift caused by run isolation SHALL be documented as expected or investigated as a regression
-