diff --git a/internal/skilldoc/source_cards_test.go b/internal/skilldoc/source_cards_test.go index 2f76def..dd4c51f 100644 --- a/internal/skilldoc/source_cards_test.go +++ b/internal/skilldoc/source_cards_test.go @@ -24,3 +24,60 @@ func TestChannelCardDisambiguatesFlashcatWorkspace(t *testing.T) { } } } + +// TestTemplateCardUpdateSemanticsArePartial pins the one fact this card has now had +// backwards in both directions. POST /template/update binds every channel field as +// *string and writes only the non-nil ones, so a channel absent from the request keeps +// its stored body and only an explicit empty string clears it. The card once promised +// omission preserved (true today, false then), was corrected to warn that omission +// cleared (true then, false today), and this test exists so the correction does not +// outlive the server behaviour that motivated it. +func TestTemplateCardUpdateSemanticsArePartial(t *testing.T) { + body, err := os.ReadFile("../../skills/flashduty/reference/template.md") + if err != nil { + t.Fatal(err) + } + text := string(body) + + // Claims that are false against the current server, in any phrasing. + for _, banned := range []string{ + // the destructive reading, retired when update became a partial update + "full-object replace", + "is CLEARED", + "every channel field you do not pass is written", + // buildTemplateUpdates covers 14 channel-content fields, not 16. + "16 channel fields", + // status is not a field of update's request at all, so it is not something + // that "survives" omission alongside the pointer-typed inputs. + "and `status` survive omission", + } { + if strings.Contains(text, banned) { + t.Errorf("template card asserts %q; update writes only the fields the request carries", banned) + } + } + + for _, want := range []string{ + // the semantics, stated as a partial update + "partial update", + "leaves it alone", + // clearing is now an explicit act, and the sharp edge is that older builds + // dropped an empty-string flag before it reached the wire, making a clear a + // silent no-op. Both halves have to stay on the card. + "explicit empty string", + "v1.4.2", + "14 channel-content fields", + "feishu_app_card_v2_table_enabled", + "info --json", + // 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..b71d71a 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` writes only the fields you send.** A channel you omit keeps its current +content; a channel you send as an empty string is cleared. So a one-channel edit sends one +channel — but read the clearing caveat under Gotchas before you try to empty one. + ## 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,41 @@ 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` patches, so you send only the channel you are changing. What still bites is the +*body*: carry it with `jq --rawfile` and `--data -`, never through `"$(...)"`. ```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. Pull the current source of the channel you are changing +fduty template info "$T" --json > /tmp/tpl.json +jq -r '.feishu_app' /tmp/tpl.json > /tmp/feishu_app.tpl +# …edit /tmp/feishu_app.tpl… + +# 2. 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 - + +# 3. Write that one channel. NEVER move the body through "$(...)": command substitution +# strips every trailing newline, so a body ending in a blank line is silently shortened. +jq -n --rawfile feishu_app /tmp/feishu_app.tpl \ + --arg t "$T" --arg n "" \ + '{template_id:$t, template_name:$n, feishu_app:$feishu_app}' \ + | fduty template update --data - + +# 4. Verify the body round-tripped byte-for-byte — cmp catches a silent truncation that +# "the field is still non-empty" would not. +fduty template info "$T" --json | jq -r '.feishu_app' | cmp - /tmp/feishu_app.tpl \ + && echo "round-trip OK" ``` +Every channel you did not name is untouched — that is the server contract now, not luck. +Verify the body you wrote anyway: `cmp` is what separates "wrote the right bytes" from +"wrote something non-empty". + ### create @@ -166,8 +196,30 @@ 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 partial update: omitting a field leaves it alone.** The server writes + only what the request contains, so naming one channel rewrites that channel and nothing + else. All 14 channel-content fields plus `description`, `team_id`, + `feishu_app_card_v2_table_enabled` and `incident_card_hidden_fields` behave this way. + (`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.) `--template-name` is still required on every update even when unchanged. +- **To CLEAR a channel you must send it as an explicit empty string** — omitting it now + means "keep", not "clear". `--dingtalk-app ''` is the intent, but a flag set to the empty + string was dropped before it reached the wire in `fduty` **older than v1.4.2**, which + makes clearing a silent no-op on those builds. Check `fduty --version` first; if it is + older, clear via `--data` with the field spelled out — `--data '{"template_id":"…", + "template_name":"…","dingtalk_app":""}'` — and confirm with `info --json` that the + channel actually went empty. +- **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.