fix: reject victorialogs raw rows query missing start/end args

[ysyneu]

Root cause

newMonitQueryRowsCmd's RunE (internal/cli/monit_query.go) only validated that --ds-type, --ds-name, --expr were non-empty and that --args entries parsed as KEY=VALUE. It never inspected the values of --args, so fduty monit-query rows --ds-type victorialogs --args victorialogs.type=raw with no victorialogs.start/victorialogs.end was dispatched straight to the backend. VictoriaLogs raw mode requires those two args, but the backend silently returns null (HTTP 200, exit 0) instead of an error when they're missing, and the CLI just echoed it through.

Evidence

Audit run audit-20260715T031903Z-auto, session sess_5JSe2TeGpyKMTQAW8pGAxS: the agent hit this exact combination ~10+ times (transcript steps 42-83, repeated null/[] results) before it happened to find the relevant reference doc.

Fix

Client-side guard in newMonitQueryRowsCmd's RunE, right after --args parsing and before dispatch: if --ds-type victorialogs and --args victorialogs.type=raw are set but victorialogs.start or victorialogs.end is missing, return a clear error instead of calling the backend. Mirrors the existing required-flags check in the same function. Scope is intentionally narrow — only this one known-bad combination is validated, matching what the transcript evidence shows; other ds-type/arg contracts (e.g. loki) are left untouched per Occam's razor.

Revision (review blocker fix)

The initial error message pointed readers to skills/flashduty/reference/monit-query.md for the start/end requirement. Review caught that this doc doesn't document victorialogs.type/.start/.end at all, and its Gotchas section states the opposite for rows in general ("no time flags — embed the range in --expr"), which would confuse rather than help. There is no reference/victorialogs.md file anywhere in this repo either. Rather than bolt on doc content to reconcile a general-vs-exception rule (out of scope for this narrow client-side fix), the doc pointer was dropped from the error string — the error text alone already states exactly what's required (--args victorialogs.start=<unix> --args victorialogs.end=<unix>), so no external reference is needed to resolve it.

RED → GREEN

RED (before the fix):

=== RUN   TestMonitQueryRowsVictorialogsRawRequiresTimeRange
    monit_query_test.go:278: expected error to mention victorialogs.start/end, got "flashduty: decoding data into *flashduty.QueryRowsResponse (request_id test-request-id): json: cannot unmarshal object into Go value of type flashduty.QueryRowsResponse"
    monit_query_test.go:281: rows should not have been called: 1 request(s)
--- FAIL: TestMonitQueryRowsVictorialogsRawRequiresTimeRange (0.00s)

(stub.requests == 1 confirms the call was dispatched to the backend before the fix, matching the silent-passthrough root cause.)

GREEN (after adding the RunE guard, and after dropping the doc pointer in the revision):

=== RUN   TestMonitQueryRowsVictorialogsRawRequiresTimeRange
--- PASS: TestMonitQueryRowsVictorialogsRawRequiresTimeRange (0.00s)

Final verification (fresh, post-revision)

• go test ./internal/cli/... -run TestMonitQueryRows -v — all PASS (Flags, HappyPath, RequiredFlags x3, InvalidArgs, VictorialogsRawRequiresTimeRange)
• go build ./... — OK
• go vet ./... — OK
• gofmt -l internal/cli/monit_query.go — no output (clean)
• git diff --stat against the prior commit — only internal/cli/monit_query.go, 1 line changed (removed the doc-pointer suffix from the error string); no other files touched
• CI on the revised push (commit 0dcc519): lint, build (ubuntu-latest), build (macos-latest), build (windows-latest), Analyze (go), Analyze (actions), CodeQL — all PASS. PR is MERGEABLE/CLEAN against feat/ai-sre.

Test plan

• RED test added and confirmed failing against pre-fix code
• GREEN after the RunE guard
• No regressions in the surrounding monit-query rows/diagnose test suite
• go build/go vet/gofmt clean
• Review blocker (misleading/nonexistent doc pointer) resolved by removing the pointer; error text is self-sufficient

[ysyneu]

Closing after tracing the contract ownership end to end.

The public OpenAPI and the go-flashduty version already pinned by feat/ai-sre allow VictoriaLogs raw queries to provide either victorialogs.start + victorialogs.end or victorialogs.timespan.value + victorialogs.timespan.unit. This PR only accepts the first form, so it incorrectly rejects a valid SDK/OpenAPI request and duplicates API semantics in the CLI layer.

The underlying null/HTTP-200 failure is addressed at the service boundary in monit-webapi: 31f8f37 validates both time-range forms and maps Edge errors to real HTTP error statuses; aa22cb8 extends the shared query-rows validator. The CLI should keep using go-flashduty as its contract boundary instead of maintaining a narrower copy here.

Keeping the branch for traceability; no code from this PR should merge.