From 45c43bcda469378c8836632634af1bf0f942459b Mon Sep 17 00:00:00 2001 From: ysyneu Date: Fri, 21 Aug 2026 20:35:15 -0700 Subject: [PATCH 1/2] docs(skill): template update replaces the whole object, not just passed fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit POST /template/update binds all 16 channel fields plus description as plain strings and writes every one of them unconditionally, so a channel absent from the request is stored as "". The template card claimed the opposite — that omitted channel flags are left unchanged — which turns a one-channel edit into a silent wipe of every other channel on the template, for every escalation rule bound to it. The published OpenAPI description already states the correct behavior ("Replace the content of every channel"); the card contradicted it. - state the destructive semantics and name the only inputs that really are patch-semantics: team_id, feishu_app_card_v2_table_enabled, incident_card_hidden_fields, status - replace the update hot flow with snapshot -> edit -> preview -> write-all -> verify-field-set; checking only the edited field cannot detect the damage, which always lands on the fields the caller did not touch - fix the flag name in the pointer-semantics gotcha: the CLI spells it --feishu-app-card-v2-table-enabled - warn that `list` returns every channel's full source per row (it has no --fields projection) and give the file+jq form instead Guard test bans the retracted claim from returning and pins the corrected text. --- internal/skilldoc/source_cards_test.go | 36 ++++++++++++++++ skills/flashduty/reference/template.md | 60 +++++++++++++++++++++++--- 2 files changed, 89 insertions(+), 7 deletions(-) diff --git a/internal/skilldoc/source_cards_test.go b/internal/skilldoc/source_cards_test.go index 2f76def..7831ae5 100644 --- a/internal/skilldoc/source_cards_test.go +++ b/internal/skilldoc/source_cards_test.go @@ -24,3 +24,39 @@ 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", + } { + 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", + "info --json", + "Verify the FIELD SET", + "--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..edaed2a 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,46 @@ 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 + +# 1. Snapshot the whole template — this is both your backup and your write payload +fduty template info "$T" --json > /tmp/tpl.json +NONEMPTY='to_entries[]|select(.value|type=="string")|select(.value!="")|.key' +jq -r "$NONEMPTY" /tmp/tpl.json # the channels that must survive this edit + +# 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 +fduty template preview --type feishu_app --content "$(cat /tmp/feishu_app.tpl)" \ + --incident-id + +# 4. Write — your edit PLUS every other channel that was non-empty in the snapshot +fduty template update "$T" \ + --template-name "$(jq -r '.template_name // ""' /tmp/tpl.json)" \ + --description "$(jq -r '.description // ""' /tmp/tpl.json)" \ + --feishu-app "$(cat /tmp/feishu_app.tpl)" \ + --dingtalk-app "$(jq -r '.dingtalk_app // ""' /tmp/tpl.json)" \ + --dingtalk "$(jq -r '.dingtalk // ""' /tmp/tpl.json)" +# …one flag per non-empty key from step 1; omitting any of them clears it + +# 5. Verify the FIELD SET, not just your edit +fduty template info "$T" --json > /tmp/tpl_after.json +diff <(jq -r "$NONEMPTY" /tmp/tpl.json | sort) <(jq -r "$NONEMPTY" /tmp/tpl_after.json | sort) +# empty diff = nothing was wiped. A key only on the left = you just deleted a live +# channel; restore it immediately 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. + ### create @@ -166,8 +201,19 @@ 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 16 channel 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 `team_id`, `feishu_app_card_v2_table_enabled`, + `incident_card_hidden_fields` and `status` survive omission (they are patch-semantics). + Always snapshot with `info --json` first and pass every non-empty channel back — see the + hot flow above. `--template-name` is required on every update even when unchanged. +- **`--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. From 2ceaf7e42d8f4a403dced39b2d1b0448fbfad4ce Mon Sep 17 00:00:00 2001 From: ysyneu Date: Fri, 21 Aug 2026 20:54:28 -0700 Subject: [PATCH 2/2] docs(skill): stop routing template bodies through command substitution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of the previous commit found the safe-write flow it introduced carried the same class of defect it exists to prevent. Three corrections: Silent truncation. The flow moved channel bodies with `"$(cat …)"` / `"$(jq -r …)"`. Bash command substitution strips every trailing newline, so a body that legitimately ends in a blank line was written back shortened — and the verification step compared only which fields were non-empty, so a truncated-but-still-non-empty channel reported clean. Measured: a 15-byte body round-trips as 11. The write step now builds the whole request with `jq --rawfile` (byte-exact) and posts it via `--data -`, and step 5 diffs per-channel byte lengths instead of the non-empty field set, which catches truncation and wipes alike. Rebuilding the body from the snapshot also removes the previous "one flag per non-empty key" instruction, whose jq filter was unscoped and surfaced template_id/status/created_at/updated_at — keys `update` has no flags for. Miscount. `buildTemplateUpdates` writes 14 channel-content fields, not 16. Category error. `status` was listed among the inputs that survive omission. It is not a field of `update`'s request at all — it moves only through the separate enable/disable endpoints, which the CLI does not expose. The survivors are exactly the pointer-typed inputs: team_id, feishu_app_card_v2_table_enabled, incident_card_hidden_fields. Guard extended and mutation-verified. --- internal/skilldoc/source_cards_test.go | 16 ++++++- skills/flashduty/reference/template.md | 65 +++++++++++++++----------- 2 files changed, 54 insertions(+), 27 deletions(-) diff --git a/internal/skilldoc/source_cards_test.go b/internal/skilldoc/source_cards_test.go index 7831ae5..2e14096 100644 --- a/internal/skilldoc/source_cards_test.go +++ b/internal/skilldoc/source_cards_test.go @@ -41,6 +41,11 @@ func TestTemplateCardUpdateSemanticsAreDestructive(t *testing.T) { 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) @@ -51,8 +56,17 @@ func TestTemplateCardUpdateSemanticsAreDestructive(t *testing.T) { "full-object replace", "is CLEARED", "survive omission", + "14 channel-content fields", "info --json", - "Verify the FIELD SET", + // 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) { diff --git a/skills/flashduty/reference/template.md b/skills/flashduty/reference/template.md index edaed2a..549dec0 100644 --- a/skills/flashduty/reference/template.md +++ b/skills/flashduty/reference/template.md @@ -57,38 +57,43 @@ verify. Skipping step 1 or step 5 is how a live channel gets silently blanked. ```bash 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 -NONEMPTY='to_entries[]|select(.value|type=="string")|select(.value!="")|.key' -jq -r "$NONEMPTY" /tmp/tpl.json # the channels that must survive this edit +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 -fduty template preview --type feishu_app --content "$(cat /tmp/feishu_app.tpl)" \ - --incident-id - -# 4. Write — your edit PLUS every other channel that was non-empty in the snapshot -fduty template update "$T" \ - --template-name "$(jq -r '.template_name // ""' /tmp/tpl.json)" \ - --description "$(jq -r '.description // ""' /tmp/tpl.json)" \ - --feishu-app "$(cat /tmp/feishu_app.tpl)" \ - --dingtalk-app "$(jq -r '.dingtalk_app // ""' /tmp/tpl.json)" \ - --dingtalk "$(jq -r '.dingtalk // ""' /tmp/tpl.json)" -# …one flag per non-empty key from step 1; omitting any of them clears it - -# 5. Verify the FIELD SET, not just your edit +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 "$NONEMPTY" /tmp/tpl.json | sort) <(jq -r "$NONEMPTY" /tmp/tpl_after.json | sort) -# empty diff = nothing was wiped. A key only on the left = you just deleted a live -# channel; restore it immediately from /tmp/tpl.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. +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. @@ -202,13 +207,21 @@ Note: `create` / `update` flags use **hyphenated** names (`--dingtalk-app`, `--f - **`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` is a full-object replace — every channel field you omit is CLEARED.** The - server writes all 16 channel 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 `team_id`, `feishu_app_card_v2_table_enabled`, - `incident_card_hidden_fields` and `status` survive omission (they are patch-semantics). - Always snapshot with `info --json` first and pass every non-empty channel back — see the - hot flow above. `--template-name` is required on every update even when unchanged. + 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