[object Object]

← back to Cli Printing Press

fix(cli): expand pm_stale timestamp-field list for non-updated_at APIs (#1461)

55f5b26b0a45e30c3917bd6b14c400e0c5450dad · 2026-05-15 18:04:04 -0700 · Trevin Chow

* fix(cli): expand pm_stale timestamp-field list for non-updated_at APIs

Generated `internal/cli/pm_stale.go` queried only `$.updatedAt` and
`$.updated_at`, so any API using a different modification-timestamp
field returned zero stale items even when the local store had
thousands of stale rows. Confirmed with Asana (modified_at); Notion
(last_edited_time), Stripe (updated), and other variants share the
same shape.

Centralise the field list in a single `staleTimestampFields` slice and
build the WHERE/ORDER BY clauses from it, so a future addition lands in
one place. Cover the historical pair plus modified_at, last_modified,
last_edited_time, modifiedAt, lastEditedTime, updated, last_updated.

A regression test in generator_test pins the field list and the
build-from-constant approach.

Closes #1391

* fix(cli): COALESCE pm_stale ORDER BY across timestamp fields

Greptile flagged the multi-column ORDER BY: SQLite ASC sorts NULLs
first, so a row whose only timestamp is e.g. modified_at would rank
above rows that populate updatedAt because the leading ORDER BY column
sees NULL on the first row but a real value on the second. The fix:
use COALESCE so each row sorts by its first non-null timestamp,
preserving the oldest-first guarantee across mixed-API stores.

Also extend the test field list to include updatedDate (was already
in the template but missing from the assertion) and pin the
COALESCE-based ORDER BY shape so a future refactor can't silently
regress to a multi-column sort.

Refs #1391

* fix(cli): typeof-gate pm_stale WHERE for integer-typed timestamps

Greptile flagged that the WHERE predicate compared json_extract output
to an RFC 3339 string cutoff. SQLite's type-class ordering makes any
JSON number unconditionally less than any text value, so APIs that
store modification timestamps as Unix epoch seconds (Stripe's
`updated`) would have every row falsely satisfy the predicate.

Gate each per-field comparison with typeof():
- text values compare against an RFC 3339 cutoff
- integer/real values compare against a Unix epoch cutoff

The Go-side scan loop now type-switches on the parsed JSON value so
days_since is computed correctly from both string and numeric inputs.

Refs #1391

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

Files touched

Diff

commit 55f5b26b0a45e30c3917bd6b14c400e0c5450dad
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Fri May 15 18:04:04 2026 -0700

    fix(cli): expand pm_stale timestamp-field list for non-updated_at APIs (#1461)
    
    * fix(cli): expand pm_stale timestamp-field list for non-updated_at APIs
    
    Generated `internal/cli/pm_stale.go` queried only `$.updatedAt` and
    `$.updated_at`, so any API using a different modification-timestamp
    field returned zero stale items even when the local store had
    thousands of stale rows. Confirmed with Asana (modified_at); Notion
    (last_edited_time), Stripe (updated), and other variants share the
    same shape.
    
    Centralise the field list in a single `staleTimestampFields` slice and
    build the WHERE/ORDER BY clauses from it, so a future addition lands in
    one place. Cover the historical pair plus modified_at, last_modified,
    last_edited_time, modifiedAt, lastEditedTime, updated, last_updated.
    
    A regression test in generator_test pins the field list and the
    build-from-constant approach.
    
    Closes #1391
    
    * fix(cli): COALESCE pm_stale ORDER BY across timestamp fields
    
    Greptile flagged the multi-column ORDER BY: SQLite ASC sorts NULLs
    first, so a row whose only timestamp is e.g. modified_at would rank
    above rows that populate updatedAt because the leading ORDER BY column
    sees NULL on the first row but a real value on the second. The fix:
    use COALESCE so each row sorts by its first non-null timestamp,
    preserving the oldest-first guarantee across mixed-API stores.
    
    Also extend the test field list to include updatedDate (was already
    in the template but missing from the assertion) and pin the
    COALESCE-based ORDER BY shape so a future refactor can't silently
    regress to a multi-column sort.
    
    Refs #1391
    
    * fix(cli): typeof-gate pm_stale WHERE for integer-typed timestamps
    
    Greptile flagged that the WHERE predicate compared json_extract output
    to an RFC 3339 string cutoff. SQLite's type-class ordering makes any
    JSON number unconditionally less than any text value, so APIs that
    store modification timestamps as Unix epoch seconds (Stripe's
    `updated`) would have every row falsely satisfy the predicate.
    
    Gate each per-field comparison with typeof():
    - text values compare against an RFC 3339 cutoff
    - integer/real values compare against a Unix epoch cutoff
    
    The Go-side scan loop now type-switches on the parsed JSON value so
    days_since is computed correctly from both string and numeric inputs.
    
    Refs #1391
    
    ---------
    
    Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
---
 internal/generator/generator_test.go               | 53 +++++++++++++
 .../generator/templates/workflows/pm_stale.go.tmpl | 90 +++++++++++++++++++---
 2 files changed, 133 insertions(+), 10 deletions(-)

diff --git a/internal/generator/generator_test.go b/internal/generator/generator_test.go
index 9e191a45..7019a313 100644
--- a/internal/generator/generator_test.go
+++ b/internal/generator/generator_test.go
@@ -10265,6 +10265,59 @@ func TestBrowserLikeUserAgentTemplates(t *testing.T) {
 	}
 }
 
+// TestStaleTemplateCoversCommonTimestampFields pins the timestamp-key
+// list in pm_stale.go.tmpl so APIs that use modified_at (Asana),
+// last_edited_time (Notion), updated (Stripe), and similar variants
+// don't silently return zero stale items. The WHERE/ORDER BY/scan
+// loop all read from the same constant, so testing the constant is
+// the cheap-and-correct way to guard the bug.
+func TestStaleTemplateCoversCommonTimestampFields(t *testing.T) {
+	t.Parallel()
+	data, err := os.ReadFile(filepath.Join("templates", "workflows", "pm_stale.go.tmpl"))
+	require.NoError(t, err)
+	body := string(data)
+
+	for _, field := range []string{
+		"updatedAt",
+		"updated_at",
+		"updatedDate",
+		"modifiedAt",
+		"modified_at",
+		"lastModified",
+		"last_modified",
+		"lastEditedTime",
+		"last_edited_time",
+		"updated",
+		"last_updated",
+	} {
+		assert.Contains(t, body, `"`+field+`"`,
+			"pm_stale.go.tmpl must list %q as a stale-timestamp field; %s users return zero results otherwise",
+			field, field)
+	}
+
+	// Pin the dynamic-build approach so the WHERE/ORDER BY/scan stay in
+	// lockstep with the field list — a future maintainer adding a field
+	// must update one place, not three.
+	assert.Contains(t, body, "buildStaleTimestampPredicate()",
+		"pm_stale.go.tmpl should build SQL from staleTimestampFields instead of hardcoding columns")
+
+	// Pin the COALESCE-based ORDER BY so a future refactor doesn't
+	// regress to a multi-column sort, which SQLite's NULL-first ASC
+	// ordering would corrupt for rows from APIs that don't populate the
+	// leading field.
+	assert.Contains(t, body, `"COALESCE(" + strings.Join(coalesceArgs, ", ") + ") ASC"`,
+		"pm_stale.go.tmpl ORDER BY must COALESCE across staleTimestampFields, not sort multi-column")
+
+	// Pin the typeof-gated WHERE so integer-typed timestamps (Stripe's
+	// `updated`) compare against an epoch cutoff rather than the
+	// RFC 3339 cutoff. Without the gate SQLite's type-class ordering
+	// makes every numeric row unconditionally match the predicate.
+	assert.Contains(t, body, "typeof(",
+		"pm_stale.go.tmpl WHERE must gate string/numeric comparisons with typeof()")
+	assert.Contains(t, body, "cutoffEpoch",
+		"pm_stale.go.tmpl must bind a numeric cutoff alongside the RFC 3339 cutoff for integer-typed timestamps")
+}
+
 // TestSearchTemplateEmitsEmptyJSONEnvelope pins the contract: the
 // generated `search` command in --json (or piped) mode must always emit
 // a valid JSON envelope, including on no matches. Agents pipe stdout
diff --git a/internal/generator/templates/workflows/pm_stale.go.tmpl b/internal/generator/templates/workflows/pm_stale.go.tmpl
index acf4e689..ff6e54be 100644
--- a/internal/generator/templates/workflows/pm_stale.go.tmpl
+++ b/internal/generator/templates/workflows/pm_stale.go.tmpl
@@ -6,12 +6,73 @@ package cli
 import (
 	"encoding/json"
 	"fmt"
+	"strings"
 	"time"
 
 	"{{modulePath}}/internal/store"
 	"github.com/spf13/cobra"
 )
 
+// staleTimestampFields lists every JSON key the press considers a
+// modification timestamp when looking for stale items. Order matters:
+// the WHERE/ORDER BY and the Go-side scan iterate this list in order,
+// so the first key present on a row wins. Includes the historical
+// updatedAt/updated_at pair plus variants commonly seen in mainstream
+// APIs (Asana modified_at, Notion last_edited_time, Stripe updated, etc.)
+// so a `stale` query against any of them returns real data instead of
+// silently empty results.
+var staleTimestampFields = []string{
+	"updatedAt",
+	"updated_at",
+	"updatedDate",
+	"modifiedAt",
+	"modified_at",
+	"lastModified",
+	"last_modified",
+	"lastEditedTime",
+	"last_edited_time",
+	"updated",
+	"last_updated",
+}
+
+// buildStaleTimestampPredicate emits SQL clauses that match a row whose
+// modification timestamp is older than cutoff and sort rows by the
+// earliest non-null timestamp the row exposes.
+//
+// Each field needs two placeholders because the JSON value can be a
+// text RFC 3339 string (most APIs: Asana modified_at, Notion
+// last_edited_time, etc.) or a numeric Unix epoch (Stripe `updated`).
+// SQLite's type-class ordering puts numbers unconditionally below text,
+// so without a typeof gate a numeric `updated` would always satisfy
+// `... < <text-cutoff>` and every Stripe row would falsely show stale.
+// The caller binds (cutoffStr, cutoffEpoch) per field in order.
+//
+// The ORDER BY uses COALESCE rather than a multi-column sort because
+// SQLite places NULLs first under ASC, so a multi-column list would
+// float rows from APIs that don't populate the leading field above
+// rows that do, breaking the oldest-first guarantee in any mixed-API
+// store. Within a single API the value type is consistent, so COALESCE
+// produces stable per-API chronological ordering; mixed-type stores
+// still group by type-class but stay deterministic.
+//
+// Centralising the field list keeps WHERE / ORDER BY / in-Go scan in
+// lockstep.
+func buildStaleTimestampPredicate() (whereClause, orderClause string, placeholderPairs int) {
+	whereParts := make([]string, 0, len(staleTimestampFields))
+	coalesceArgs := make([]string, 0, len(staleTimestampFields))
+	for _, f := range staleTimestampFields {
+		extract := fmt.Sprintf("json_extract(data, '$.%s')", f)
+		whereParts = append(whereParts, fmt.Sprintf(
+			"((typeof(%s) = 'text' AND %s < ?) OR (typeof(%s) IN ('integer','real') AND %s < ?))",
+			extract, extract, extract, extract,
+		))
+		coalesceArgs = append(coalesceArgs, extract)
+	}
+	return strings.Join(whereParts, " OR "),
+		"COALESCE(" + strings.Join(coalesceArgs, ", ") + ") ASC",
+		len(whereParts)
+}
+
 func newStaleCmd(flags *rootFlags) *cobra.Command {
 	var days int
 	var team string
@@ -46,20 +107,23 @@ the specified number of days. Useful for identifying forgotten or blocked work.`
 			}
 			defer db.Close()
 
-			cutoff := time.Now().AddDate(0, 0, -days).Format(time.RFC3339)
+			cutoffTime := time.Now().AddDate(0, 0, -days)
+			cutoffStr := cutoffTime.Format(time.RFC3339)
+			cutoffEpoch := cutoffTime.Unix()
 
-			// Query resources table for items not updated since cutoff
-			query := `SELECT id, resource_type, data FROM resources
-				WHERE json_extract(data, '$.updatedAt') < ?
-				   OR json_extract(data, '$.updated_at') < ?`
-			qArgs := []any{cutoff, cutoff}
+			whereClause, orderClause, n := buildStaleTimestampPredicate()
+			query := "SELECT id, resource_type, data FROM resources WHERE (" + whereClause + ")"
+			qArgs := make([]any, 0, n*2+4)
+			for i := 0; i < n; i++ {
+				qArgs = append(qArgs, cutoffStr, cutoffEpoch)
+			}
 
 			if team != "" {
 				query += ` AND (json_extract(data, '$.teamId') = ? OR json_extract(data, '$.team_id') = ? OR json_extract(data, '$.team') = ?)`
 				qArgs = append(qArgs, team, team, team)
 			}
 
-			query += ` ORDER BY json_extract(data, '$.updatedAt') ASC, json_extract(data, '$.updated_at') ASC`
+			query += " ORDER BY " + orderClause
 
 			// Get total count first
 			countQuery := "SELECT COUNT(*) FROM (" + query + ")"
@@ -112,11 +176,17 @@ the specified number of days. Useful for identifying forgotten or blocked work.`
 
 				updatedAt := ""
 				daysSince := days
-				for _, key := range []string{"updatedAt", "updated_at", "updatedDate"} {
+				for _, key := range staleTimestampFields {
 					if v, ok := obj[key]; ok {
 						updatedAt = fmt.Sprintf("%v", v)
-						if t, err := time.Parse(time.RFC3339, updatedAt); err == nil {
-							daysSince = int(now.Sub(t).Hours() / 24)
+						switch ts := v.(type) {
+						case string:
+							if t, err := time.Parse(time.RFC3339, ts); err == nil {
+								daysSince = int(now.Sub(t).Hours() / 24)
+							}
+						case float64:
+							// JSON numbers decode to float64 (Stripe-style Unix epoch seconds).
+							daysSince = int(now.Sub(time.Unix(int64(ts), 0)).Hours() / 24)
 						}
 						break
 					}

← 3bb6d37b fix(cli): doctor preserves configured User-Agent when API us  ·  back to Cli Printing Press  ·  feat(cli): add ElevenLabs generation support (#1307) 5f43de50 →