# Incident Todo Action Model Repair

## Table of Contents

- [Incident Todo Action Model Repair](#incident-todo-action-model-repair)
  - [Table of Contents](#table-of-contents)
  - [Introduction](#introduction)
  - [Motivation or Background](#motivation-or-background)
  - [Detailed Design](#detailed-design)
    - [Scope](#scope)
    - [Current State](#current-state)
    - [Target Data Model](#target-data-model)
    - [Database Migration](#database-migration)
    - [Entity and API Shape](#entity-and-api-shape)
    - [Service Semantics](#service-semantics)
    - [Feishu Task Relationship](#feishu-task-relationship)
    - [Rollout Plan](#rollout-plan)
  - [Test Design](#test-design)
    - [Functional Tests](#functional-tests)
    - [Scenario Tests](#scenario-tests)
    - [Compatibility Tests](#compatibility-tests)
  - [Impacts & Risks](#impacts--risks)
  - [Unresolved Questions](#unresolved-questions)

## Introduction

This proposal repairs the Incident postmortem todo action data model so every action has a clear owner record and can be created, listed, updated, and synchronized predictably. The immediate goal is to fix the broken linkage between the current PostMortem tab and todo action rows by making `incident_id` the stable required parent, while treating `postmortem_id` as optional until the Feishu postmortem document is formally represented by `incident_postmortem` rows.

## Motivation or Background

DM-12753 reports that the incident todo action workflow is broken and blocks postmortem follow-up closure. The current API routes are shaped as postmortem child resources, for example `/incidents/{incident_id}/postmortems/{postmortem_id}/todoactions`, but the backend stores and queries todo actions only by `incident_id`.

The frontend currently does not load postmortem records from `incident_postmortem`. `management-portal-ui/src/pages/Incidents/IncidentPostMortem/index.tsx` receives the incident from `useGetIncident()`, checks `incident.feishu_url`, and displays the PostMortem tab when that URL points to a Feishu doc or wiki. The todo action API calls pass `postmortem_id = 1` as a placeholder because the backend ignores this parameter today. Therefore an empty `incident_postmortem` table is expected for the current UI flow.

This mismatch creates several concrete problems:

- The generated API requires a `postmortem_id`, but the UI has no real postmortem id and passes `1`.
- A todo action created from the PostMortem tab is effectively incident-scoped.
- Feishu status sync is incident-scoped, which matches current storage but conflicts with the route shape.
- Updating `persons` is accepted by the API request but ignored by the service update path.
- Status values are inconsistent across comments, defaults, and Feishu sync output.

The fix should focus on the core workflow fields required to make todo action ownership and status reliable. Fields such as action `type`, optional Jira linkage, and strict postmortem-row ownership can be added later, after the base incident-scoped workflow is corrected or after the postmortem document model is migrated into `incident_postmortem`.

## Detailed Design

### Scope

In scope:

- Keep todo action ownership stable with required `incident_id`.
- Add optional `postmortem_id` only as a future-compatible link to `incident_postmortem`.
- Preserve the existing Feishu postmortem document flow based on `incident.feishu_url`.
- Normalize the core action fields:
  - `id`
  - `postmortem_id`
  - `incident_id`
  - `title`
  - `type`
  - `description`
  - `linked_jira_issue`
  - `persons`
  - `deadline`
  - `status`
  - `feishu_task_id`
- Fix create, list, get, update, delete, and Feishu sync semantics around the corrected model.
- Avoid backfilling historical rows to fake postmortem ids when `incident_postmortem` has no corresponding records.

Out of scope for this change:

- Redesigning Feishu Task as the source of truth.
- Requiring a non-null `postmortem_id`.
- Creating `incident_postmortem` rows automatically for every existing `incident.feishu_url`.
- Changing the frontend information architecture away from the current `incident.feishu_url` based PostMortem tab.

### Current State

The current entity includes `PostMortemId`, but the GORM model does not:

```go
type IncidentPostMortemToDoAction struct {
    Id           uint64
    IncidentId   *uint64
    PostMortemId *uint64
    Persons      []*user.User
    Deadline     *time.Time
    Title        *string
    Description  *string
    Status       *string
    FeishuTaskId *string
}
```

The current database model only has `IncidentId`:

```go
type IncidentPostMortemToDoAction struct {
    Updatable
    IncidentId   *uint64
    Persons      *string
    Deadline     *time.Time
    Status       *string
    Title        *string
    Description  *string
    FeishuTaskId *string
}
```

Because of this, `CreateIncidentPostMortemToDoActionItem()` receives `PostMortemId` but drops it, and `ListIncidentPostMortemToDoActions()` queries by `incident_id`. This is internally consistent with the current frontend because it has no real postmortem id. The inconsistency is the public API shape, which advertises a postmortem-scoped resource even though the active product flow is incident-scoped with a Feishu document URL.

### Target Data Model

Update `models.IncidentPostMortemToDoAction` to support an optional `PostMortemId` while keeping `IncidentId` required:

```go
type IncidentPostMortemToDoAction struct {
    Updatable

    IncidentId    uint64     `gorm:"column:incident_id;type:bigint(20) unsigned;not null;index"`
    PostMortemId  *uint64    `gorm:"column:postmortem_id;type:bigint(20) unsigned;index"`
    Persons       *string    `gorm:"column:persons;type:varchar(2000)"`
    Deadline      *time.Time `gorm:"column:deadline;type:timestamp;default null"`
    Status        *string    `gorm:"column:status;type:varchar(200);default null"`
    Title         *string    `gorm:"column:title;type:varchar(200);default null"`
    Type          *string    `gorm:"column:type;type:varchar(200);default null"`
    Description   *string    `gorm:"column:description;type:varchar(2000);default null"`
    LinkedJiraIssue *string  `gorm:"column:linked_jira_issue;type:varchar(200);default null"`
    FeishuTaskId  *string    `gorm:"column:feishu_task_id;type:varchar(2000)"`
}
```

Notes:

- `incident_id` is the canonical parent for the current todo action workflow.
- `postmortem_id` is optional and should only be set when the action is truly attached to an `incident_postmortem` row.
- The current PostMortem tab should keep using incident-scoped actions until it starts loading a real postmortem row.
- `description` should be widened from `varchar(200)` to `varchar(2000)` because action descriptions can contain concrete follow-up context.
- `type` and `linked_jira_issue` are optional metadata fields. They should not affect ownership, filtering, or Feishu task sync in this delivery.
- `persons` remains a space-separated email string in this iteration to minimize migration risk. A normalized owner mapping table can be considered separately.

Status should be normalized to the following values:

```go
const (
    IncidentTodoActionStatusOpen       = "open"
    IncidentTodoActionStatusInProgress = "in_progress"
    IncidentTodoActionStatusDone       = "done"
    IncidentTodoActionStatusCanceled   = "canceled"
)
```

Creation should default to `open` when the request does not specify a status. The service should reject unknown status values before writing to the database.

### Database Migration

Add a new migration record under `migrate/record_*.go`.

Schema changes:

```sql
ALTER TABLE incident_postmortem_todoaction
    ADD COLUMN postmortem_id bigint(20) unsigned NULL AFTER incident_id,
    MODIFY COLUMN description varchar(2000) DEFAULT NULL,
    ADD INDEX idx_incident_postmortem_todoaction_postmortem_id (postmortem_id),
    ADD INDEX idx_incident_postmortem_todoaction_incident_id (incident_id);
```

Backfill strategy:

1. For rows with `postmortem_id IS NOT NULL`, leave them unchanged.
2. Do not synthesize `postmortem_id = 1`.
3. Do not backfill from `incident_id` when `incident_postmortem` is empty.
4. If a future data audit finds incidents with exactly one real `incident_postmortem` row and linked todo actions, those rows can be backfilled in a separate migration.

The column should remain nullable in this delivery. Making it non-null depends on a separate migration that creates or imports real postmortem records.

### Entity and API Shape

Keep `entity.IncidentPostMortemToDoAction` aligned with the persisted model:

```go
type IncidentPostMortemToDoAction struct {
    Id           uint64       `json:"id"`
    IncidentId   *uint64      `json:"incident_id"`
    PostMortemId *uint64      `json:"postmortem_id"`
    Persons      []*user.User `json:"persons"`
    Deadline     *time.Time   `json:"deadline"`
    Title        *string      `json:"title"`
    Type         *string      `json:"type"`
    Description  *string      `json:"description"`
    LinkedJiraIssue *string   `json:"linked_jira_issue"`
    Status       *string      `json:"status"`
    FeishuTaskId *string      `json:"feishu_task_id"`
}
```

Request and response contracts:

- Create request keeps `persons`, `deadline`, `title`, `type`, `description`, `linked_jira_issue`, `status`, and `create_feishu_task`.
- `incident_id` should come from the route.
- `postmortem_id` from the route is optional compatibility input. Ignore placeholder value `1` unless a real row exists.
- List response should include both `incident_id` and nullable `postmortem_id`.
- Get/update/delete should validate that the action belongs to the route's `incident_id`. Validate `postmortem_id` only when it is a real persisted link.

Recommended service signatures:

```go
ListIncidentPostMortemToDoActions(ctx context.Context, incidentId uint64, postMortemId *uint64) ([]*entity.IncidentPostMortemToDoAction, error)
GetIncidentPostMortemToDoActionItem(ctx context.Context, incidentId uint64, postMortemId *uint64, id uint64) (*entity.IncidentPostMortemToDoAction, error)
UpdateIncidentPostMortemToDoActionItem(ctx context.Context, incidentId uint64, postMortemId *uint64, id uint64, actionUpdate *EditIncidentPostMortemToDoActionParams) error
DeleteIncidentPostMortemToDoActionItem(ctx context.Context, incidentId uint64, postMortemId *uint64, id uint64) error
SyncToDoActionStatusFromFeishu(ctx context.Context, incidentId uint64, postMortemId *uint64) error
```

If changing interface signatures creates too much blast radius, introduce new internal helper methods with `postMortemId` first, then update callers in one PR.

### Service Semantics

Create:

- Parse `incident_id` from the route.
- Parse `postmortem_id` from the route as optional compatibility input.
- If `postmortem_id` points to a real postmortem row, verify it belongs to the incident and persist it.
- If `postmortem_id` is the current placeholder value `1` and no real row exists, persist only `incident_id`.
- Convert `persons` emails to users, preserving the existing validation behavior.
- Default status to `open`.
- Save required `incident_id` and optional `postmortem_id`.
- If `create_feishu_task` is true, create the Feishu task and store `feishu_task_id`.

List:

- Primary current query: `WHERE incident_id = ?`.
- If a real `postmortem_id` is provided and exists, narrow to `WHERE incident_id = ? AND postmortem_id = ?`.
- Do not treat placeholder `postmortem_id = 1` as a real filter.

Get/update/delete:

- Query by both `id` and `incident_id`.
- If the action has a non-null `postmortem_id`, route handlers should verify it matches the route postmortem id.
- Update should include `persons`, `deadline`, `title`, `description`, and `status`.
- Unknown statuses should return invalid argument errors.

Sync:

- Query by `incident_id`.
- If a real `postmortem_id` exists for the current page, sync only that postmortem's actions.
- For each row with `feishu_task_id`, fetch the Feishu task.
- Map Feishu completion state to the normalized local status:
  - completed task -> `done`
  - incomplete task -> keep existing `open` or `in_progress`; default to `open` if current status is empty
- Sync `title`, `description`, `deadline`, and status.
- Do not overwrite `persons` from Feishu until Feishu executor semantics are reviewed.

### Feishu Task Relationship

The local todo action remains the source of truth for incident/postmortem ownership. Feishu Task is an optional external execution surface.

Create behavior:

- Local action creation with `create_feishu_task=false` should work without Feishu.
- Local action creation with `create_feishu_task=true` should fail if Feishu task creation fails, preserving current behavior.

Update behavior:

- If `feishu_task_id` is set, update the Feishu task after the local action update succeeds.
- If Feishu update fails, return an error so the caller can retry. A later enhancement can introduce async reconciliation.

Delete behavior:

- Delete the local action.
- If `feishu_task_id` is set, attempt to delete the Feishu task.
- If Feishu delete fails after the local delete, log the error with action id and task id. This avoids resurrecting deleted local rows.

### Rollout Plan

1. Add migration for nullable `postmortem_id`, description width, indexes, and compatibility-safe no-op backfill.
2. Update model/entity/converters.
3. Update service methods to require and validate `incident_id`; support nullable `postmortem_id`.
4. Update controller route handlers to pass both `incident_id` and optional `postmortem_id`.
5. Normalize status constants and request validation.
6. Update frontend callers only if a real postmortem id is introduced. Otherwise keep the compatibility path and document that `1` is a placeholder.
7. Regenerate API docs with `make generate` if request/response comments change.
8. Run targeted unit tests and a manual scenario test on incident postmortem todo actions.

## Test Design

### Functional Tests

Backend unit tests should cover:

- Create persists required `incident_id`.
- Create persists `postmortem_id` only when the route id maps to a real postmortem row.
- Create defaults missing status to `open`.
- Create rejects invalid status.
- List filters by `incident_id` for the current Feishu-doc based PostMortem tab.
- List narrows by `postmortem_id` only when a real postmortem row exists.
- Get/update/delete reject actions that do not belong to the route's incident.
- Update persists `persons`.
- Converters round-trip `PostMortemId`, `IncidentId`, `Persons`, and `FeishuTaskId`.
- Feishu sync processes actions under the target incident, or under the target real postmortem when present.

Migration tests should cover:

- Existing rows keep null `postmortem_id`.
- Migration does not write placeholder `postmortem_id = 1`.
- Optional postmortem rows can be linked in a later migration.

### Scenario Tests

Manual or integration scenarios:

- Create a PostMortem-tab todo action without Feishu task and verify it appears under the incident's PostMortem tab.
- Create a postmortem todo action with Feishu task and verify `feishu_task_id` is stored.
- Update owner, deadline, title, description, and status, then reload the postmortem page.
- Mark a Feishu task complete, run sync, and verify local status becomes `done`.
- If real postmortem rows are introduced, create two postmortems under one incident and verify actions do not bleed between them.
- Open incident detail and verify linked action count and incomplete count can be computed from `incident_id`.

### Compatibility Tests

Upgrade compatibility:

- Old rows with only `incident_id` must remain readable after migration.
- The current frontend's placeholder `postmortem_id = 1` must keep working.
- Real postmortem-scoped list should be enabled only after the frontend has a real postmortem id.

Downgrade compatibility:

- The migration adds a nullable column and widens `description`, so old binaries should ignore `postmortem_id`.
- If old binaries create rows without `postmortem_id` during rollback, the rows remain compatible with the backfill strategy.

External compatibility:

- Feishu Task API usage remains the same for create/get/update/delete.
- No Jira integration is introduced by this design.

## Impacts & Risks

Impacts:

- Todo action ownership becomes explicit at incident level and remains compatible with the current frontend.
- Future postmortem-row ownership can be added without another table rewrite.
- Incident detail can still aggregate actions through `incident_id`.
- Status values become predictable for UI display, filtering, and incomplete-count calculation.

Risks:

- If the backend starts strictly validating `postmortem_id` immediately, the current frontend placeholder id will break todo actions.
- Feishu update/delete failures can leave local and external state temporarily inconsistent.
- Normalizing status values can require frontend display changes if it currently expects `Ongoing`, `ongoing`, or `InProgress`.

Mitigations:

- Keep `postmortem_id` nullable for the first rollout.
- Keep compatibility behavior for placeholder `postmortem_id = 1`.
- Add real postmortem validation only when the frontend loads or creates real postmortem rows.
- Keep a small status compatibility mapper at the API boundary during migration if existing clients need it.

## Unresolved Questions

- Should the product create an `incident_postmortem` row when users set `incident.feishu_url`, or should the Feishu URL remain an incident-level field?
- Should deleting a local todo action delete the associated Feishu task or only unlink it?
- Should Feishu incomplete tasks always map to `open`, or should existing `in_progress` be preserved?
- When should action `type` and `linked_jira_issue` be added: in the same delivery after the core model is fixed, or in a follow-up DM?

  | Track | Todo Action | Priority | Type | Deadline | Status | Owner 判断 |
  |---|---|---:|---|---|---|---|
  | TiKV failover | TiKV 支持 I/O hang 场景 fast fail，避免节点半死不活时 leader 长时间不切换 | P1 | Prevention / Kernel | TBD | Open | TiKV owner，文档 Action Items 已指派 |
  | PD slow store | Slow store score 支持多节点 slow/hang 场景的调度决策，不只处理单点慢节点 | P1/P2 | Prevention / Scheduler | TBD | Open | PD owner，文档中已 mention slow store 相
  关 owner |
  | AZ HA 内核 | 定义单 AZ 故障下 TiKV/TiFlash/PD/TiDB 的 HA 行为预期和 failover SLA | P1 | Design / Prevention | TBD | Open | Kernel + HA owner |
  | AZ HA 管控 | 单 AZ 故障时的管控面 mitigation 能力：批量识别故障 AZ 节点、批量 evict leader、必要时关闭 EC2 触发 raft failover | P1 | Automation / Ops | TBD | Open | TiDB Cloud /
  TiDB-X / Ops owner |
  | Incident response | 故障响应流程优化：告警风暴识别、War Room、Incident Owner 交接、实时 Dashboard、云厂商事件订阅 | P0 | Process / Automation | 2026/05/30 | Open | 文档已有
  owner，并且子项已有多个责任人 |
  | Drill | 单 AZ 故障演练，包含交接演练 | P1 | Drill / Validation | 2026/05/22 | Open | 文档已有 owner |
  | TiKV broadcast | 消除 TiKV 广播机制对非故障 AZ 的影响 | P2 | Kernel / Isolation | TBD | Open | TiKV owner |
  | AWS diagnostics | AWS 网络诊断能力增强：周期性可达性探测、临时开启 VPC Flow Log API、OPS 集成 | P2 | Observability / Ops | TBD | Open | Cloud infra / Ops owner |
  | 3AZ balancing | 评估并消除 3AZ balancing blocking，依赖单 AZ 演练结果 | P2 | Control Plane / HA | TBD | Blocked by drill | TiDB-X / Control plane owner |
  | Component isolation | 增加组件/资源之间的变更隔离开关，避免 TiDB、TiFlash 等组件间变更互相阻塞 | P2 | Control Plane | TBD | Open | TiDB-X / Control plane owner |
  | Dedicated drill | Dedicated TiKV/TiFlash 单 AZ failure 演练 | P1 | Drill / Validation | TBD | Open | 文档已有 owner |
  | Premium drill | Premium TiKV/TiFlash 单 AZ failure 演练 | P2 | Drill / Validation | TBD | Open | 需要补 owner |
  | PD transfer leader | PD 提供 transfer-leader limit 评估方法，并支持大型集群更高 transfer-leader 速度 | P1 | Scheduler / Performance | TBD | Open | PD owner，和 5/9 文档 action 重
  复，建议合并跟踪 |
  | Alert fix | 修复失效的 IOPS 告警规则 | P1 | Observability | 2026/05/15 | Done | 已有文档链接和 owner |
  | Raft memory leak | 修复 raft 引起的内存泄漏问题，跟踪 tikv/issues#19593 | P1 | Bugfix | TBD | Open / Tracking | TiKV/Raft owner |
  | Frontline comms | 和前线的信息共享机制：客户影响、AWS 状态、内部处理进展统一同步 | P1/P2 | Process | TBD | Open | Incident response owner / TAM / Support |