Skip to content

Commit 059ee7e

Browse files
committed
fix(cli): anchor defaulted --since at --until; note --fields everywhere it does not apply
A defaulted lookback --since (24h, or 30d for open-only incident and active alert lists) now spans back from the parsed --until instead of from now. With --until +1d the 30d default used to produce a 31-day window, which the list endpoints reject; a past --until with the 24h default produced an inverted window. timeutil.Lookback recognizes the past-relative duration form (and rejects "+" offsets, which the day shorthand expansion would otherwise accept). The "--fields does not apply" note moves to the root PersistentPostRun, driven by whether a projection ran. Commands that print their own output (version, config show) and table mode now get the note too, instead of ignoring --fields silently.
1 parent 631075f commit 059ee7e

9 files changed

Lines changed: 92 additions & 11 deletions

File tree

‎internal/cli/command.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,6 @@ func (ctx *RunContext) PrintTotal(items any, cols []output.Column, total int) er
8080

8181
// WriteResult prints a success message as plain text or JSON.
8282
func (ctx *RunContext) WriteResult(message string) {
83-
noteFieldsNotApplied(ctx.Cmd)
8483
writeResult(ctx.Writer, message)
8584
}
8685

@@ -89,7 +88,6 @@ func (ctx *RunContext) WriteResult(message string) {
8988
// shell redirection (`> file.csv`) captures the bytes verbatim instead of the
9089
// canned "OK: POST ..." acknowledgment.
9190
func (ctx *RunContext) WriteRaw(body []byte) error {
92-
noteFieldsNotApplied(ctx.Cmd)
9391
_, err := ctx.Writer.Write(body)
9492
return err
9593
}
@@ -102,7 +100,6 @@ func (ctx *RunContext) WriteResultJSON(data any, humanMessage string) error {
102100
_, _ = fmt.Fprintln(ctx.Writer, humanMessage)
103101
return nil
104102
}
105-
noteFieldsNotApplied(ctx.Cmd)
106103
out, err := marshalStructured(data)
107104
if err != nil {
108105
return fmt.Errorf("failed to marshal output: %w", err)

‎internal/cli/gen_support.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,7 @@ func printGenericResult(ctx *RunContext, data any) error {
358358
return err
359359
}
360360
noteFieldsMissing(ctx.Cmd, proj)
361+
fieldsApplied = true
361362
data = proj.out
362363
if pp, ok := ctx.Printer.(projectingPrinter); ok {
363364
projected := *ctx

‎internal/cli/outputfields.go‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,10 +116,17 @@ func noteFieldsMissing(cmd *cobra.Command, p projection) {
116116
}
117117
}
118118

119-
// noteFieldsNotApplied says --fields left a command's structured output as it
120-
// was: an acknowledgement or a raw (file/CSV) body has no record to project.
119+
// fieldsApplied records that the global --fields projected something during
120+
// the current run; the root resets it before each command runs.
121+
var fieldsApplied bool
122+
123+
// noteFieldsNotApplied runs after every command (root PersistentPostRun) and
124+
// says so when --fields was given but projected nothing: table output, an
125+
// acknowledgement, a raw (file/CSV) body, or a command that prints its own
126+
// text (version, config show). One hook covers every output path, so no
127+
// command can ignore the flag silently.
121128
func noteFieldsNotApplied(cmd *cobra.Command) {
122-
if flagFields != "" && currentOutputFormat().Structured() {
129+
if flagFields != "" && !fieldsApplied {
123130
_, _ = fmt.Fprintf(cmd.ErrOrStderr(), "note: --fields does not apply to the output of %q; printed unchanged\n", cmd.CommandPath())
124131
}
125132
}
@@ -150,5 +157,6 @@ func (p projectingPrinter) Print(data any, columns []output.Column) error {
150157
return err
151158
}
152159
noteFieldsMissing(p.cmd, proj)
160+
fieldsApplied = true
153161
return p.inner.Print(proj.out, nil)
154162
}

‎internal/cli/outputfields_test.go‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,11 +127,32 @@ func TestGlobalFieldsOnAcknowledgement(t *testing.T) {
127127
if !strings.Contains(stdout, `"message"`) {
128128
t.Errorf("acknowledgement should print unchanged, got %q", stdout)
129129
}
130-
if !strings.Contains(stderr, `note: --fields does not apply to the output of "flashduty channel disable"; printed unchanged`) {
130+
if !strings.Contains(stderr, `note: --fields does not apply to the output of "flashduty channel disable"`) {
131131
t.Errorf("want not-applied note, got %q", stderr)
132132
}
133133
}
134134

135+
// TestGlobalFieldsOnSelfPrintedOutput: commands that print their own output
136+
// (version, config show) and table mode say --fields did not apply instead of
137+
// ignoring it silently.
138+
func TestGlobalFieldsOnSelfPrintedOutput(t *testing.T) {
139+
for _, args := range [][]string{
140+
{"version", "--fields", "version", "--output-format", "json"},
141+
{"config", "show", "--fields", "base_url"},
142+
{"member", "list", "--fields", "member_id"},
143+
} {
144+
saveAndResetGlobals(t)
145+
newGFStub(t)
146+
_, stderr, err := execCommandSplit(args...)
147+
if err != nil {
148+
t.Fatalf("%v: %v", args, err)
149+
}
150+
if !strings.Contains(stderr, "note: --fields does not apply to the output of") {
151+
t.Errorf("%v: want not-applied note, got %q", args, stderr)
152+
}
153+
}
154+
}
155+
135156
// TestLocalFieldsFlagShadowsGlobal: a command whose own --fields is a request
136157
// field sends it and does not project its output.
137158
func TestLocalFieldsFlagShadowsGlobal(t *testing.T) {

‎internal/cli/root.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ var rootCmd = &cobra.Command{
6161
}
6262
updateNotice = nil
6363
updateCheckWarning = ""
64+
fieldsApplied = false
6465
if cmd.CommandPath() == cmd.Root().Name()+" update" {
6566
return nil
6667
}
@@ -89,6 +90,7 @@ var rootCmd = &cobra.Command{
8990
return nil
9091
},
9192
PersistentPostRun: func(cmd *cobra.Command, _ []string) {
93+
noteFieldsNotApplied(cmd)
9294
if updateCheckWarning != "" {
9395
_, _ = fmt.Fprintf(cmd.ErrOrStderr(), "\n%s\n", updateCheckWarning)
9496
}

‎internal/cli/window.go‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,21 @@ var nowFn = time.Now
4242
// parseWindow parses a curated verb's --since/--until pair into unix seconds
4343
// and announces the effective window on stderr (see noteWindow). why, when
4444
// non-empty, says which default produced the window.
45+
//
46+
// A defaulted lookback --since ("24h", "30d") spans back from --until, not
47+
// from now: with --until moved, a now-relative default would stretch the
48+
// window past the server's 31-day cap (future --until) or invert it (past
49+
// --until). With --until at its "now" default the two readings coincide.
4550
func parseWindow(cmd *cobra.Command, since, until, why string) (start, end int64, err error) {
46-
start, err = timeutil.Parse(since)
47-
if err != nil {
48-
return 0, 0, fmt.Errorf("invalid --since: %w", err)
49-
}
5051
end, err = timeutil.Parse(until)
5152
if err != nil {
5253
return 0, 0, fmt.Errorf("invalid --until: %w", err)
5354
}
55+
if span, ok := timeutil.Lookback(since); ok && !cmd.Flags().Changed("since") {
56+
start = end - int64(span/time.Second)
57+
} else if start, err = timeutil.Parse(since); err != nil {
58+
return 0, 0, fmt.Errorf("invalid --since: %w", err)
59+
}
5460
noteWindow(cmd.ErrOrStderr(), time.Unix(start, 0), time.Unix(end, 0), why)
5561
return start, end, nil
5662
}

‎internal/cli/window_test.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,12 @@ func TestOpenStateDefaultWindow(t *testing.T) {
147147
{"alert active", []string{"alert", "list", "--active"}, 30 * day, true},
148148
{"alert unfiltered", []string{"alert", "list"}, day, false},
149149
{"alert active explicit since", []string{"alert", "list", "--active", "--since", "1h"}, 3600, false},
150+
// A defaulted --since spans back from --until, so a moved --until
151+
// neither stretches the window past 30d nor inverts it.
152+
{"incident triggered future until", []string{"incident", "list", "--progress", "Triggered", "--until", "+1d"}, 30 * day, true},
153+
{"alert active future until", []string{"alert", "list", "--active", "--until", "+1d"}, 30 * day, true},
154+
{"incident past until", []string{"incident", "list", "--until", "2d"}, day, false},
155+
{"incident explicit since and until", []string{"incident", "list", "--since", "3d", "--until", "1d"}, 2 * day, false},
150156
}
151157
for _, tc := range cases {
152158
t.Run(tc.name, func(t *testing.T) {

‎internal/timeutil/parse.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,21 @@ func Parse(s string) (int64, error) {
8484
return 0, fmt.Errorf("unable to parse time %q: expected duration (24h), RFC3339 (2006-01-02T15:04:05Z07:00), date (2006-01-02), datetime (2006-01-02 15:04:05), or unix timestamp in seconds or milliseconds", s)
8585
}
8686

87+
// Lookback reports the span of a past-relative duration ("24h", "30d") — the
88+
// form Parse reads as "now minus duration". ok is false for every other form
89+
// (absolute times, "now", "+" future offsets).
90+
func Lookback(s string) (d time.Duration, ok bool) {
91+
s = strings.TrimSpace(s)
92+
if strings.HasPrefix(s, "+") {
93+
return 0, false
94+
}
95+
d, err := time.ParseDuration(expandDays(s))
96+
if err != nil || d < 0 {
97+
return 0, false
98+
}
99+
return d, true
100+
}
101+
87102
// msThreshold is the magnitude cutoff separating a unix timestamp in seconds
88103
// from one in milliseconds: 100,000,000,000 as seconds is the year 5138, so
89104
// any input at or above it is treated as milliseconds instead.

‎internal/timeutil/parse_test.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,3 +250,28 @@ func TestParse(t *testing.T) {
250250
})
251251
}
252252
}
253+
254+
// TestLookback: only past-relative durations are lookbacks; "+" future
255+
// offsets (which Atoi-based day expansion would otherwise accept), "now",
256+
// dates and unix timestamps are not.
257+
func TestLookback(t *testing.T) {
258+
cases := map[string]struct {
259+
want time.Duration
260+
ok bool
261+
}{
262+
"24h": {24 * time.Hour, true},
263+
"30d": {30 * 24 * time.Hour, true},
264+
" 90m ": {90 * time.Minute, true},
265+
"now": {0, false},
266+
"+1d": {0, false},
267+
"+24h": {0, false},
268+
"2026-04-01": {0, false},
269+
"1712000000": {0, false},
270+
}
271+
for in, tc := range cases {
272+
got, ok := Lookback(in)
273+
if got != tc.want || ok != tc.ok {
274+
t.Errorf("Lookback(%q) = %v, %v; want %v, %v", in, got, ok, tc.want, tc.ok)
275+
}
276+
}
277+
}

0 commit comments

Comments
 (0)