From 8da199e555051c314bef88b81e2ebca1fa3a0e1b Mon Sep 17 00:00:00 2001 From: ysyneu Date: Sat, 15 Aug 2026 23:03:28 -0700 Subject: [PATCH] chore(skills): drop legacy rows guidance from command cards Skill cards guide new usage, so a deprecated verb earns no card entry: - skilldoc dump now excludes commands with a cobra Deprecated marker (the command stays in the CLI tree with its runtime warning). - cligen reads the spec's deprecated flag and stamps generated commands; monit query-rows carries the 'use monit-query data instead' message. - monit-query / monit cards now teach only data (query_result.v1) and diagnose; the raw rows passthrough remains callable during the migration period but is no longer agent guidance. --- internal/cli/zz_generated_diagnostics.go | 5 +-- internal/cmd/cligen/main.go | 44 +++++++++++++++++------ internal/skilldoc/build.go | 7 +++- skills/flashduty/reference/monit-query.md | 14 ++------ skills/flashduty/reference/monit.md | 17 +++------ 5 files changed, 48 insertions(+), 39 deletions(-) diff --git a/internal/cli/zz_generated_diagnostics.go b/internal/cli/zz_generated_diagnostics.go index 432b31f..64702e0 100644 --- a/internal/cli/zz_generated_diagnostics.go +++ b/internal/cli/zz_generated_diagnostics.go @@ -270,8 +270,9 @@ func genDiagnosticsQueryRowsCmd() *cobra.Command { var fDsType string var fExpr string cmd := &cobra.Command{ - Use: "query-rows", - Short: "Query data source rows", + Use: "query-rows", + Short: "Query data source rows", + Deprecated: "use 'monit-query data' instead", Long: `Query data source rows. Deprecated. Run a synchronous ad-hoc query and return the historical flattened rows shape. Existing consumers should migrate to '/monit/query/data', which preserves frames, records, and samples without forcing every result into legacy rows. diff --git a/internal/cmd/cligen/main.go b/internal/cmd/cligen/main.go index 7915994..4290861 100644 --- a/internal/cmd/cligen/main.go +++ b/internal/cmd/cligen/main.go @@ -150,6 +150,9 @@ type specOp struct { // envelope. RespTree then holds the ROW (array-element) fields, and help must // document the output as a TOP-LEVEL array (`jq '.[]'`), not `under data`. RespArray bool + // Deprecated is the cobra deprecation message when the spec marks the + // operation `deprecated: true`; empty means the operation is current. + Deprecated string } // schemaField is one node in a (possibly nested) request/response schema tree. @@ -244,18 +247,27 @@ func collectServices(paths, schemas map[string]any, sdk map[string]map[string]me respType = mi.RespType } respTree, respArray := walker.responseTree(e.op, respType) + opDeprecated := "" + if ok, _ := e.op["deprecated"].(bool); ok { + if msg := opDeprecationMessages[opID]; msg != "" { + opDeprecated = msg + } else { + opDeprecated = "this API operation is deprecated" + } + } svc.Ops = append(svc.Ops, specOp{ - OpID: opID, - Method: method, - HTTP: e.http, - Path: e.path, - Summary: str(e.op, "summary"), - Desc: str(e.op, "description"), - Example: walker.example(e.op), - Fields: walker.fields(e.op), - ReqTree: walker.requestTree(e.op), - RespTree: respTree, - RespArray: respArray, + OpID: opID, + Method: method, + HTTP: e.http, + Path: e.path, + Summary: str(e.op, "summary"), + Desc: str(e.op, "description"), + Example: walker.example(e.op), + Fields: walker.fields(e.op), + ReqTree: walker.requestTree(e.op), + RespTree: respTree, + RespArray: respArray, + Deprecated: opDeprecated, }) } if len(svc.Ops) > 0 { @@ -972,6 +984,13 @@ var opAliases = map[string][]string{ "channelInfo": {"get", "detail"}, } +// opDeprecationMessages maps a spec-deprecated operationId to its cobra +// deprecation message. Ops without an entry get a generic message when the +// spec marks them deprecated. +var opDeprecationMessages = map[string]string{ + "monit-read-query-rows": "use 'monit-query data' instead", +} + // positional describes the positional argument a generated command exposes. type positional struct { Wire string // request-body wire key the positional folds into @@ -1181,6 +1200,9 @@ func emitCmd(fn string, s service, o specOp, mi methodInfo) string { fmt.Fprintf(&b, "\tcmd := &cobra.Command{\n") fmt.Fprintf(&b, "\t\tUse: %q,\n", use) fmt.Fprintf(&b, "\t\tShort: %q,\n", oneLine(o.Summary)) + if o.Deprecated != "" { + fmt.Fprintf(&b, "\t\tDeprecated: %q,\n", o.Deprecated) + } fmt.Fprintf(&b, "\t\tLong: %s,\n", quoteMultiline(longHelp(o, scalars, complexFields, specByWire))) if aliases := opAliases[o.OpID]; len(aliases) > 0 { quoted := make([]string, len(aliases)) diff --git a/internal/skilldoc/build.go b/internal/skilldoc/build.go index 23be5e1..5d669a8 100644 --- a/internal/skilldoc/build.go +++ b/internal/skilldoc/build.go @@ -52,7 +52,12 @@ func walk(c *cobra.Command, parents []string, d *Dump) { // command gains both real behavior of its own AND subcommands — this // predicate must change to emit a card for that command's own behavior // while still not treating its children as absent. - if !c.HasSubCommands() && c.Runnable() && !c.Hidden { + // + // Deprecated commands are excluded: cards guide NEW usage, and a + // deprecated verb's replacement is what the card prose should teach. + // The command stays in the CLI tree (with its runtime deprecation + // warning) for the migration period — it just earns no card entry. + if !c.HasSubCommands() && c.Runnable() && !c.Hidden && c.Deprecated == "" { d.Commands = append(d.Commands, command(c, path)) } diff --git a/skills/flashduty/reference/monit-query.md b/skills/flashduty/reference/monit-query.md index 691df13..7af2477 100644 --- a/skills/flashduty/reference/monit-query.md +++ b/skills/flashduty/reference/monit-query.md @@ -12,7 +12,6 @@ Prereq: `SKILL.md` read. Datasource-side RCA: query a monitoring datasource dire |---|---| | pre-clustered RCA evidence (log patterns / metric trends) | `diagnose --operation log_patterns\|metric_trends` | | run a query and get natural structured results (frames / records / samples) | `data --expr ""` | -| legacy flattened rows (migration only) | `rows --expr ""` | ## Hot flow — diagnose a noisy datasource @@ -50,27 +49,18 @@ Pre-clustered RCA findings (log_patterns or metric_trends) - `--timeout-seconds` int - response: single object (`data` unwrapped to the top level) — fields: data_handling (object); ds_name (string); ds_type (string); operation (string); query (string); results (array); schema_version (string); window (object) -### rows -Raw datasource passthrough (returns values/rows as the datasource itself would). Deprecated — prefer 'monit-query data' -- `--args` stringSlice -- `--ds-name` string -- `--ds-type` string -- `--expr` string -- response: TOP-LEVEL array — pipe `--json | jq '.[]'` (NOT `.items[]`) — fields: fields (object); values (object) - ## Key concepts -- **`data` = structured query.** Stable `query_result.v1` response: dispatch on `result.kind` — `frames` (typed tables / time series), `records` (schema-flexible rows, big ints as decimal strings), `samples` (instant samples with labels; non-finite floats as `"NaN"` / `"+Inf"` / `"-Inf"`). Prefer this over `rows`. -- **`rows` = legacy raw passthrough (deprecated).** Numeric fields under `values` (metric canonical key `__value__`); labels/columns under `fields`. Only for callers that still need the flattened shape during migration. **Time belongs in the query expression**, not in flags. +- **`data` = structured query.** Stable `query_result.v1` response: dispatch on `result.kind` — `frames` (typed tables / time series), `records` (schema-flexible rows, big ints as decimal strings), `samples` (instant samples with labels; non-finite floats as `"NaN"` / `"+Inf"` / `"-Inf"`). - **`diagnose` = pre-clustered evidence.** Its versioned response echoes the datasource, query, and RFC 3339 analysis window. Each result contains method-specific `pattern_evidence` (logs) or `series_evidence` (metrics), structured window statistics, and observations; log results also declare redaction and untrusted observed-data paths in `data_handling`. Takes `--time-start` / `--time-end` (relative like `-1h`, `now`, or unix seconds). ## Gotchas - **Discover the datasource name first** (`monit datasource-list`). A wrong/guessed name 400s `can not find datasource` — re-list, don't retry variants. - **A 5xx or HTML-body error is TRANSIENT** — retry the same call ≤3×. Do NOT fall back to SSH, `monit-agent`, or incident search on a transient datasource error. -- **`data` and `rows` have no time flags** — putting `--time-start` on them is wrong; embed the range in `--expr` (on `data`, `--delay-seconds` shifts the point-in-time lookback). +- **`data` has no time flags** — putting `--time-start` on it is wrong; embed the range in `--expr` (or use `--delay-seconds` for the point-in-time lookback). - Empty results = the query genuinely matched nothing in that window — report it, don't widen blindly. - **`diagnose` rejects windows wider than 6 hours outright.** `--time-start`/`--time-end` span is capped at 6h server-side; the default window is the last 15 minutes (`--time-start 15m`, `--time-end now`). Widen within the cap, don't retry past it. - **`--ds-type` on `diagnose` only accepts `prometheus`, `victorialogs`, `loki`, `mysql`.** `monit datasource-list` can return other types (e.g. `oracle`, `postgres`, `clickhouse`, `elasticsearch`, `sls`) — those are not supported here. diff --git a/skills/flashduty/reference/monit.md b/skills/flashduty/reference/monit.md index 4de07f5..1382c30 100644 --- a/skills/flashduty/reference/monit.md +++ b/skills/flashduty/reference/monit.md @@ -28,7 +28,7 @@ Prereq: `SKILL.md` read. **SKILL.md + this card = full competence on monitors | export / import rules (backup/migrate) | `rule-export` / `rule-import` | | what datasource types support rules | `rule-dstypes` | | per-channel / per-node / total counters | `rule-counter-channel` / `rule-counter-node` / `rule-counter-total` | -| run ad-hoc PromQL / SQL / LogQL | `query-rows` | +| run ad-hoc PromQL / SQL / LogQL | `monit-query data` (curated; see the monit-query card) | | log-pattern / metric-trend RCA evidence | `query-diagnose` | | list monitored hosts/targets | `targets` | | what tools a target exposes | `tools-catalog` | @@ -43,7 +43,8 @@ fduty monit datasource-list --output-format toon fduty monit datasource-list --type prometheus --output-format toon # 2a. point-in-time query (PromQL/SQL/LogQL); ALL time range goes INSIDE --expr -fduty monit query-rows --ds-type prometheus --ds-name \ +# (the curated 'monit-query data' — see the monit-query card) +fduty monit-query data --ds-type prometheus --ds-name \ --expr 'rate(http_requests_total{job="api"}[5m])' --output-format toon # 2b. log pattern RCA over last 15 min (time_range via --data; omit = last 15 min default) @@ -166,16 +167,6 @@ Diagnose data source - body-only (`--data`): input (object) (required); methods (array); options (object); time_range (object) - response: single object (`data` unwrapped to the top level) — fields: data_handling (object); ds_name (string); ds_type (string); operation (string); query (string); results (array); schema_version (string); window (object) -### query-rows -Query data source rows -- `--account-id` int64 — Optional consistency check. Must equal the authenticated account when supplied; mismatched values are rejected. Business execution always uses the authenticated account. -- `--delay-seconds` int64 — Look-back offset in seconds applied to point-in-time queries (Prometheus, Loki stats, VictoriaLogs stats). Ignored for raw / detail queries. -- `--ds-name` string (required) — Data source name; must match a configured data source under the tenant. -- `--ds-type` string (required) — Data source type; must match a configured data source under the tenant. Examples: 'prometheus', 'loki', 'victorialogs', 'sls', 'elasticsearch', 'mysql', 'postgres', 'oracle', 'clickhouse'. -- `--expr` string (required) — Query expression. Syntax depends on 'ds_type' and is interpreted by the corresponding monit-edge client (PromQL for Prometheus, LogQL for Loki, SQL for SQL sources, etc.). -- body-only (`--data`): args (object) -- response: TOP-LEVEL array — pipe `--json | jq '.[]'` (NOT `.items[]`) — fields: fields (object); values (object) - ### rule-audit-detail Get rule audit snapshot - `--id` int64 (required) — Audit record ID — the 'id' of an audit row returned by 'POST /monit/rule/audits', NOT the rule ID. Passing a rule ID returns HTTP 400. @@ -439,7 +430,7 @@ Invoke target tools - **Datasource name is not guessable.** A `can not find datasource` 400 means the name is wrong — re-run `datasource-list` and copy the exact `Name`. Never invent variants. - **`datasource-info` (and the `datasource-create`/`datasource-update` responses) return credentials exactly as configured — nothing is masked.** The `payload` object includes whatever passwords, API keys, tokens, and similar fields were set, in the clear. Treat the response as sensitive: don't dump it into logs or chat, don't echo it back beyond what the task needs, and don't pass it on to another tool. -- **`query-rows` has no time flags.** There is no `--time-start` / `--time-end` / `--operation`. Embed all time range and bucketing inside `--expr`. Passing those flags is a silent no-op or error. +- **`monit-query data` has no time flags.** There is no `--time-start` / `--time-end` / `--operation`. Embed all time range and bucketing inside `--expr`. Passing those flags is a silent no-op or error. - **`query-diagnose` time window via `--data`**, not flags. Pass `{"time_range":{"start":,"end":},...}`. Window wider than 6 hours is rejected server-side. Omitting `time_range` defaults to the last 15 minutes. - **`rule_configs` and nested arrays require `--data`.** The queries, thresholds, enabled_times, and labels objects cannot be expressed as flat flags — pass them as inline JSON via `--data '{"rule_configs":{...}}'`. Typed scalar flags (`--name`, `--enabled`, `--cron-pattern`, `--ds-type`) override matching `--data` keys. - **`folder-id 0` is not a universal "all rules" sentinel.** If the API says "Folder not found", believe it. For global inventory use `rule-counter-status` / `rule-counter-node` first, then run `rule-list-basic` against real folder IDs only.