feat(pod): add list filters and default to running pods

[TimPietruskyRunPod]

Summary

• Add --status, --since, --created-after, and --all flags to pod list
• Default behavior now shows only running pods (like docker ps)
• Use --all or --status exited to see non-running pods
• Slim down output fields to essential info (id, name, status, image, gpu, cost)

Usage

runpodctl pod list                              # running pods only (default)
runpodctl pod list --all                        # all pods including exited
runpodctl pod list --status exited              # filter by status
runpodctl pod list --since 24h                  # pods created in last 24 hours
runpodctl pod list --created-after 2025-01-15   # pods created after date

Test plan

• Unit tests pass (go test ./cmd/pod/...)
• E2E: pod list returns only running pods
• E2E: pod list --all includes exited pods

[max4c]

PR Review: feat(pod): add list filters and default to running pods

Overall the direction is good — the docker-ps-style default and the new filter flags are a nice UX improvement. There are several issues worth addressing before merging, ranging from a breaking API regression to logic bugs and missing tests.

---

1. Breaking change: PodListOptions fields silently removed (Bug / Breaking)

The PR drops IncludeMachine and IncludeNetworkVolume from the PodListOptions passed to client.ListPods(), but those fields still exist on the struct in internal/api/pods.go and are still wired up in ListPods. The opts struct is constructed without them:

opts := &api.PodListOptions{
    ComputeType: listComputeType,
    Name:        listName,
    // IncludeMachine and IncludeNetworkVolume silently dropped
}

The flags are removed from the CLI, but the API layer is not updated. This means:

• Any caller that was relying on ?includeMachine=true or ?includeNetworkVolume=true query params now silently gets a different response.
• PodListOptions has dead fields that are never populated from the CLI anymore.

Suggested fix: Either keep the flags (and just not display the fields in podListOutput) or also clean up PodListOptions and ListPods in the API layer to remove the dead fields. Silently dropping them is the worst option.

---

2. --since and --created-after are mutually exclusive but both silently accepted (Logic Bug)

The README-style usage examples suggest these are independent filters, but the code picks the later of the two as the cutoff:

if cutoff.IsZero() || t.After(cutoff) {
    cutoff = t
}

This is not documented in any flag description and is surprising behavior. A user running:

runpodctl pod list --since 7d --created-after 2025-01-01

would silently get whichever is more restrictive, with no warning. At minimum document this in the flag help strings. Better: return an error when both are provided, or clarify in --help which takes precedence.

---

3. --since does not accept standard Go duration strings (UX / Correctness)

The custom parseDuration only supports h (hours) and d (days). Running --since 30m or --since 1h30m returns an error even though these are perfectly natural inputs.

time.ParseDuration from the standard library handles ns, us, ms, s, m, h and compound forms like 1h30m. The only missing piece is d (days). A more robust implementation:

func parseDuration(s string) (time.Duration, error) {
    // Handle days suffix manually, fall back to stdlib for everything else
    if strings.HasSuffix(s, "d") {
        n, err := strconv.Atoi(strings.TrimSuffix(s, "d"))
        if err != nil {
            return 0, fmt.Errorf("invalid duration %q: %w", s, err)
        }
        return time.Duration(n) * 24 * time.Hour, nil
    }
    return time.ParseDuration(s)
}

---

4. --status comparison is case-inconsistent with --all interaction (Logic Bug)

The statusFilter is uppercased via strings.ToUpper, but the comparison uses strings.EqualFold:

statusFilter := strings.ToUpper(listStatus)
// ...
if statusFilter != "" && !strings.EqualFold(p.DesiredStatus, statusFilter) {

Since statusFilter is already uppercased, strings.EqualFold works, but it is misleading. Either uppercase both sides explicitly or use only EqualFold without the ToUpper call. Minor, but confusing for future readers.

More importantly: --status and --all are not mutually exclusive but combining them is ambiguous. If a user runs --all --status RUNNING, the --all flag is silently ignored because statusFilter is non-empty. This should either be documented or produce a warning/error.

---

5. podListOutput.CreatedAt is typed as interface{} (Code Quality)

The podListOutput struct copies CreatedAt interface{} directly from the raw API response. This means JSON output will serialize whatever the API returns (string, number, null) with no normalization. The parseCreatedAt helper is already written — use it to normalize the field to a *time.Time or a formatted string in the output struct:

type podListOutput struct {
    // ...
    CreatedAt string `json:"createdAt,omitempty"`
}

// In the loop:
ct := parseCreatedAt(p.CreatedAt)
var createdAtStr string
if !ct.IsZero() {
    createdAtStr = ct.UTC().Format(time.RFC3339)
}
items = append(items, podListOutput{
    // ...
    CreatedAt: createdAtStr,
})

---

6. parseDuration rejects negative/zero values silently (Edge Case)

strconv.Atoi will succeed for n = 0 or negative values like -5. --since -5d or --since 0h will either produce a future cutoff (filtering out everything) or a zero cutoff (no filtering). Add an explicit check:

if n <= 0 {
    return 0, fmt.Errorf("invalid duration %q: must be a positive number", s)
}

---

7. Missing unit tests for the new filtering logic

The test file only gains flag-existence checks. There are no tests for:

• parseDuration (valid inputs: 1h, 7d; invalid: 5m, -1d, 0h, abc)
• parseCreatedAt (RFC3339 string, Unix timestamp string, invalid string, non-string type)
• runList filtering logic (status filter, --all, combined --since + --created-after)

The parseDuration and parseCreatedAt functions are pure and easily unit-testable. Given the behavioral edge cases above, these tests are important.

---

8. Minor: output.Error(err) called before return err (Consistency)

In runList, every early-return error path calls both output.Error(err) and return err. But the original code only does this for the api.NewClient() error — the client.ListPods() error path did not call output.Error in the original. The PR is inconsistent: parseDuration and time.Parse errors call output.Error, but the pattern should be uniform. Check whether the caller (cobra) also prints errors and whether double-printing is happening.

---

Summary

Severity	Issue
High	IncludeMachine/IncludeNetworkVolume silently dropped from API call
Medium	--since + --created-after interaction undocumented / surprising
Medium	parseDuration rejects standard suffixes (m, s, etc.)
Medium	--all silently ignored when --status is also provided
Medium	Missing unit tests for new pure functions and filtering logic
Low	podListOutput.CreatedAt typed as interface{} — normalise it
Low	parseDuration accepts zero/negative values
Low	output.Error call consistency

[TimPietruskyRunPod]

Addressed all review feedback:

• Dead API fields (High): Removed IncludeMachine/IncludeNetworkVolume from PodListOptions and the API layer
• parseDuration (Medium): Rewrote to support stdlib formats (30m, 1h30m) with d as the only custom suffix, plus positive-value validation
• --all + --status conflict (Medium): Returns error when both flags are used together
• Missing tests (Medium): Added table-driven unit tests for parseDuration and parseCreatedAt
• CreatedAt type (Low): Normalized from interface{} to RFC3339 string
• Redundant ToUpper (Low): Cleaned up since EqualFold handles case already

@max4c ready for another look

[max4c]

Re-review: Follow-up on Previous Feedback

Thanks for the quick turnaround. Here is a precise accounting of what was addressed, what remains, and two new issues I noticed in the updated diff.

---

Previously Raised Issues — Status

1. Dead API fields (IncludeMachine / IncludeNetworkVolume) — FIXED

The fields have been removed from PodListOptions and from the ListPods query-building logic in internal/api/pods.go. The API layer and the struct are now consistent. The e2e test that exercised IncludeMachine: true has also been removed. This is clean.

2. --since + --created-after undocumented interaction — PARTIALLY ADDRESSED

The original code silently picked the more restrictive cutoff. The updated code still does exactly that — no error is returned when both flags are supplied together, and there is no flag description explaining the precedence. Contrast this with --all + --status, which now correctly errors. The same treatment should apply here, or at minimum the flag Usage strings should document the behavior explicitly. This remains an undocumented surprise.

3. parseDuration rejecting standard suffixes (30m, 1h30m, etc.) — FIXED

The function was rewritten to fall back to time.ParseDuration for everything that does not end in d. The test cases confirm 30m and 1h30m now work correctly.

4. --all + --status conflict — FIXED

An explicit error is returned when both flags are provided together. Good.

5. Missing unit tests — FIXED

Table-driven tests for both parseDuration and parseCreatedAt are present and cover the meaningful edge cases (positive, zero, negative durations; RFC3339, Unix timestamp, invalid, nil, non-string inputs). Adequate coverage for pure functions.

6. podListOutput.CreatedAt typed as interface{} — FIXED

The field is now typed as string with omitempty. The value is normalized via parseCreatedAt and formatted as RFC3339 before being placed in the output struct. JSON output will now be consistent regardless of what the API returns.

7. parseDuration accepting zero/negative values — FIXED

Both the d-suffix branch and the stdlib fallback branch check d <= 0 and return an error. Test cases for 0h, 0d, -1d, and -2h confirm this.

8. output.Error consistency — NOT ADDRESSED

The updated code still calls output.Error(err) immediately before every return err in runList. The original client.ListPods error path does not call output.Error — it just returns the error. If Cobra (or whatever is at the call site) also prints the error, users will see each error message twice on the non-parseDuration paths. This is low-severity but worth auditing to confirm there is no double-printing in practice.

---

New Issues Noticed in the Updated Diff

A. parseCreatedAt signature accepts interface{} but the struct field is now string

podListOutput.CreatedAt was correctly changed to string, and the caller was updated to call parseCreatedAt(p.CreatedAt). However, parseCreatedAt still accepts interface{} and performs a type assertion internally. Since p.CreatedAt is a field on the API Pod struct, it would be cleaner (and more type-safe) to check what type Pod.CreatedAt actually is in internal/api/pods.go and update parseCreatedAt to accept a concrete type instead. If Pod.CreatedAt is already a string in the API struct, then the interface{} dance is unnecessary indirection.

B. --since flag description says "e.g. 1h, 7d" — 30m and compound formats are now supported but not mentioned

The flag help string is:

"filter pods created within duration (e.g. 1h, 7d)"

Now that parseDuration delegates to time.ParseDuration, inputs like 30m, 1h30m, 2h30m all work. The example in the flag description should be updated to reflect this so users discover it:

"filter pods created within duration (e.g. 30m, 2h, 1h30m, 7d)"

---

Summary Table

#	Issue	Status
1	Dead IncludeMachine/IncludeNetworkVolume API fields	Fixed
2	--since + --created-after undocumented interaction	Partially addressed — no error returned, no docs
3	parseDuration rejecting 30m, 1h30m	Fixed
4	--all + --status conflict	Fixed
5	Missing unit tests	Fixed
6	CreatedAt typed as interface{} in output struct	Fixed
7	parseDuration zero/negative values	Fixed
8	output.Error double-print risk	Not addressed
A	parseCreatedAt takes interface{} unnecessarily	New — low severity
B	--since flag description outdated	New — low severity

Most of the meaningful issues have been addressed. The two remaining actionable items are: (2) either error or document when --since and --created-after are used together, and (8) verify there is no double error printing. Items A and B are minor polish.

[max4c]

Two remaining nits:

• --since + --created-after conflict: when both are provided, the code silently picks the more restrictive value. This is inconsistent with how --all + --status is handled (which correctly returns an error). Would prefer an explicit error here too so the caller knows their input was contradictory.

• Double error output: several error paths call output.Error(err) immediately before return err. If Cobra (or anything upstream) also prints returned errors, this will double-print. Either swallow the return (return nil) after printing, or drop the output.Error call and let the caller handle display.

[max4c]

addressed comments and review lgtm

[promptless]

📝 Documentation updates detected!

Updated existing suggestion: Update runpodctl documentation for v2.0

---

Tip: Tell your friends working on non-commercial open-source projects to apply for free Promptless access at promptless.ai/oss ❤️