diff --git a/internal/skilldoc/source_cards_test.go b/internal/skilldoc/source_cards_test.go index 2f76def..2e14096 100644 --- a/internal/skilldoc/source_cards_test.go +++ b/internal/skilldoc/source_cards_test.go @@ -24,3 +24,53 @@ func TestChannelCardDisambiguatesFlashcatWorkspace(t *testing.T) { } } } + +// TestTemplateCardUpdateSemanticsAreDestructive pins the one fact the template card +// previously stated backwards. POST /template/update binds every channel field as a plain +// string and writes all of them unconditionally, so a channel absent from the request is +// stored as "" — omitting a flag deletes that channel's body for every escalation rule +// bound to the template. The card used to promise the opposite ("omitted channel flags are +// left unchanged"), which turns a one-field edit into a silent wipe of a live channel. +func TestTemplateCardUpdateSemanticsAreDestructive(t *testing.T) { + body, err := os.ReadFile("../../skills/flashduty/reference/template.md") + if err != nil { + t.Fatal(err) + } + text := string(body) + + for _, banned := range []string{ + "omitted channel flags are left unchanged", + "only supplied fields overwrite", + // buildTemplateUpdates writes 14 channel-content fields, not 16. + "16 channel fields", + // status is not a field of update's request at all, so it is not a + // pointer-typed input that "survives" omission. + "and `status` survive omission", + } { + if strings.Contains(text, banned) { + t.Errorf("template card reasserts the retracted patch-semantics claim %q; update writes every channel field unconditionally", banned) + } + } + + for _, want := range []string{ + "full-object replace", + "is CLEARED", + "survive omission", + "14 channel-content fields", + "info --json", + // Length comparison, not a non-empty field-set check: only the former catches a + // body that was truncated rather than cleared. + "Verify LENGTHS", + // The write path must move bodies with jq, never through command substitution, + // which strips every trailing newline off a template body. + "--rawfile", + "--data -", + `"$(cat`, + "strips *all* trailing newlines", + "--feishu-app-card-v2-table-enabled", + } { + if !strings.Contains(text, want) { + t.Errorf("template card missing %q", want) + } + } +} diff --git a/skills/flashduty/reference/template.md b/skills/flashduty/reference/template.md index 3d2299f..549dec0 100644 --- a/skills/flashduty/reference/template.md +++ b/skills/flashduty/reference/template.md @@ -2,6 +2,10 @@ Prereq: `SKILL.md` read. Read verbs are free. `create`, `update`, `delete` mutate account-wide notification templates — confirm before running. `delete ` is **irreversible**. +**`update` is a full-object replace.** Every channel field you do not pass is written +empty. A one-channel edit is still a whole-object write — never call `update` without the +`info --json` snapshot from the hot flow below. + ## Route here when "通知模板 / 消息模板 / 告警通知格式 / 飞书模板 / Slack 模板 / 邮件模板 / template CRUD / custom template / preview notification / validate template" → **template**. NOT `channel` (channel = escalation policy routing; template = the rendered text/card body). The key ID is **`template_id`** (string), returned by `list` or `create`. @@ -46,15 +50,51 @@ fduty template create \ fduty template info --output-format toon ``` -## Hot flow — update one channel on an existing template +## Hot flow — change one channel on an existing template + +`update` overwrites the whole object, so this is read → modify → preview → write → +verify. Skipping step 1 or step 5 is how a live channel gets silently blanked. ```bash -# template-id is POSITIONAL; --template-name is required even on update -fduty template update \ - --template-name "Critical-Feishu-v2" \ - --feishu "$(cat ./feishu-v3.tpl)" +T= # POSITIONAL on update/info/delete; --template-name always required +CHLEN='["dingtalk","dingtalk_app","email","feishu","feishu_app","slack","slack_app","sms","teams_app","telegram","voice","wecom","wecom_app","zoom"] as $ch | . as $t | $ch[] | "\(.)\t\($t[.] // "" | length)"' + +# 1. Snapshot the whole template — this is both your backup and your write payload +fduty template info "$T" --json > /tmp/tpl.json +jq -r "$CHLEN" /tmp/tpl.json # every channel and its current byte length + +# 2. Edit only the channel you care about, on disk +jq -r '.feishu_app' /tmp/tpl.json > /tmp/feishu_app.tpl +# …edit /tmp/feishu_app.tpl… + +# 3. Preview the edited source against a REAL incident before writing +jq -n --rawfile c /tmp/feishu_app.tpl \ + '{type:"feishu_app", content:$c, incident_id:""}' \ + | fduty template preview --data - + +# 4. Write — rebuild the body from the snapshot, moving template bodies with jq and +# NEVER through "$(...)": command substitution strips every trailing newline, so a +# body ending in a blank line would come back silently shortened. +jq -c --rawfile feishu_app /tmp/feishu_app.tpl \ + '{template_id, template_name, description, + dingtalk, dingtalk_app, email, feishu, feishu_app, slack, slack_app, sms, + teams_app, telegram, voice, wecom, wecom_app, zoom} + | .feishu_app = $feishu_app' /tmp/tpl.json \ + | fduty template update --data - +# Everything left out of that object is patch-semantics and survives untouched: +# team_id, feishu_app_card_v2_table_enabled, incident_card_hidden_fields. + +# 5. Verify LENGTHS, not just the field you edited +fduty template info "$T" --json > /tmp/tpl_after.json +diff <(jq -r "$CHLEN" /tmp/tpl.json) <(jq -r "$CHLEN" /tmp/tpl_after.json) +# Only the channel you edited may differ. A channel that dropped to 0 was wiped; a +# channel a few bytes shorter was truncated — restore it from /tmp/tpl.json. ``` +Checking only the field you edited is **not** verification — the damage from a full-object +replace always lands on the fields you did not touch, and comparing only *which* fields are +non-empty misses a body that was shortened rather than cleared. + ### create @@ -166,8 +206,27 @@ Note: `create` / `update` flags use **hyphenated** names (`--dingtalk-app`, `--f ## Gotchas - **`info`, `update`, `delete` take `` as a positional first argument** — pass it bare, not as `--template-id`. `create`, `list`, `preview`, `validate`, `get-preset`, `functions`, `variables` take all inputs as flags. -- **`update` replaces every channel field you pass — omitted channel flags are left unchanged** (server behavior: only supplied fields overwrite). Always pass `--template-name` even if the name is unchanged — it is required on update. -- **`--feishu-app-card-table-enabled` uses pointer semantics on `update`** — unlike the plain string channel-content flags, it patches the table-rendering setting only when the flag is explicitly passed; omit it to leave the existing setting untouched. It is a plain bool on `create` (no prior setting to preserve). +- **`update` is a full-object replace — every channel field you omit is CLEARED.** The + server writes all 14 channel-content fields plus `description` on every call, and a + field absent from the request arrives as the empty string: omitting `--dingtalk-app` + sets `dingtalk_app` to `""`, and that channel silently stops rendering for every + escalation rule bound to the template. Only the pointer-typed inputs survive omission: + `team_id`, `feishu_app_card_v2_table_enabled`, `incident_card_hidden_fields`. (`status` + is not part of `update`'s request at all — it moves only through the separate + enable/disable endpoints, which the CLI does not expose — so `update` can never change + it.) Always snapshot with `info --json` first and rebuild the body from that snapshot — + see the hot flow above. `--template-name` is required on every update even when + unchanged. +- **Never move a template body through `"$(cat …)"` or `"$(jq -r …)"`.** Bash command + substitution strips *all* trailing newlines, so a body that legitimately ends in a blank + line is written back shortened — and a check that only asks which fields are non-empty + cannot see it, because the field is still non-empty. Carry bodies with `jq --rawfile` + and write with `--data -`, as the hot flow does. +- **`--feishu-app-card-v2-table-enabled` uses pointer semantics on `update`** — unlike the plain string channel-content flags, it patches the table-rendering setting only when the flag is explicitly passed; omit it to leave the existing setting untouched. It is a plain bool on `create` (no prior setting to preserve). +- **`list` returns every channel's full template source for every row** — a few dozen + templates blow past a tool-output cap in one call. Never render it directly: go to a + file and project. `fduty template list --limit 100 --json > /tmp/tpl_list.json && jq -r + '.items[] | [.template_id, .template_name, .team_id] | @tsv' /tmp/tpl_list.json`. - **`delete` is permanent.** The built-in preset (`template_id = 000000000000000000000001`) can be addressed by that sentinel ID in `info` and `delete` — don't delete it. - **`validate` reads from a local `--file`; `preview` takes inline `--content`.** They are complementary: `validate` gives size-vs-limit diagnostics; `preview` renders against real or mock incident data. - **`email` uses `html/template` syntax; `sms` and `voice` use `text/template`** — auto-escaping rules differ. Don't mix them.