← back to Cli Printing Press
fix(cli): Store.Get propagates sql.ErrNoRows so callers can gate on existence (#1031)
66fd401bddf4e27e7de28ee19eccc88791719bef · 2026-05-11 00:25:02 -0700 · Trevin Chow
* fix(cli): Store.Get propagates sql.ErrNoRows so callers can gate on existence
The generator's Store.Get template swallowed sql.ErrNoRows into (nil, nil),
which silently bypassed `if err != nil { return err }` checks at every
novel-command call site -- a row-not-found code path could exit 0 even when
the caller intended notFoundErr.
Propagate the sentinel directly. The one in-template caller
(data_source.go.tmpl::resolveLocal) now distinguishes missing rows via
errors.Is(err, sql.ErrNoRows). Generator-level test pins the rendered
template shape and emission of the runtime test into the printed CLI;
template-level test pins the runtime contract.
Closes #962
* docs(cli): trim test godocs that restated function names
Fallback code review flagged two godocs that opened with the test name
("TestGenerateStoreGetPropagatesErrNoRows pins...", "TestGet_MissingRowReturnsErrNoRows
pins...") in violation of the AGENTS.md hygiene rule "Do not restate the
field or function name in its comment." Rewrite to lead with the invariant
each test guards.
Files touched
M internal/generator/generator_test.goM internal/generator/templates/data_source.go.tmplM internal/generator/templates/store.go.tmplM internal/generator/templates/store_schema_version_test.go.tmpl
Diff
commit 66fd401bddf4e27e7de28ee19eccc88791719bef
Author: Trevin Chow <trevin@trevinchow.com>
Date: Mon May 11 00:25:02 2026 -0700
fix(cli): Store.Get propagates sql.ErrNoRows so callers can gate on existence (#1031)
* fix(cli): Store.Get propagates sql.ErrNoRows so callers can gate on existence
The generator's Store.Get template swallowed sql.ErrNoRows into (nil, nil),
which silently bypassed `if err != nil { return err }` checks at every
novel-command call site -- a row-not-found code path could exit 0 even when
the caller intended notFoundErr.
Propagate the sentinel directly. The one in-template caller
(data_source.go.tmpl::resolveLocal) now distinguishes missing rows via
errors.Is(err, sql.ErrNoRows). Generator-level test pins the rendered
template shape and emission of the runtime test into the printed CLI;
template-level test pins the runtime contract.
Closes #962
* docs(cli): trim test godocs that restated function names
Fallback code review flagged two godocs that opened with the test name
("TestGenerateStoreGetPropagatesErrNoRows pins...", "TestGet_MissingRowReturnsErrNoRows
pins...") in violation of the AGENTS.md hygiene rule "Do not restate the
field or function name in its comment." Rewrite to lead with the invariant
each test guards.
---
internal/generator/generator_test.go | 36 ++++++++++++++++++++++
internal/generator/templates/data_source.go.tmpl | 8 +++--
internal/generator/templates/store.go.tmpl | 5 ++-
.../templates/store_schema_version_test.go.tmpl | 30 ++++++++++++++++++
4 files changed, 73 insertions(+), 6 deletions(-)
diff --git a/internal/generator/generator_test.go b/internal/generator/generator_test.go
index 357ad261..0f0c85a6 100644
--- a/internal/generator/generator_test.go
+++ b/internal/generator/generator_test.go
@@ -2141,6 +2141,42 @@ func TestGenerateStoreMigrateUsesBeginImmediate(t *testing.T) {
"migrate must read PRAGMA user_version BEFORE entering withMigrationLock so newer-DB rejection happens before lock acquisition")
}
+// Callers gating on existence rely on errors.Is(err, sql.ErrNoRows); the
+// emitted Store.Get must surface the sentinel rather than swallow it into
+// a nil-shape that bypasses the caller's err check.
+func TestGenerateStoreGetPropagatesErrNoRows(t *testing.T) {
+ t.Parallel()
+
+ apiSpec := minimalSpec("errnorows-canary")
+ outputDir := filepath.Join(t.TempDir(), naming.CLI(apiSpec.Name))
+ gen := New(apiSpec, outputDir)
+ gen.VisionSet = VisionTemplateSet{Store: true}
+ require.NoError(t, gen.Generate())
+
+ storeSrc, err := os.ReadFile(filepath.Join(outputDir, "internal", "store", "store.go"))
+ require.NoError(t, err)
+ storeCode := stripGoComments(string(storeSrc))
+
+ assert.NotRegexp(t, `(?s)func \(s \*Store\) Get\([^)]*\) \([^)]*\) \{[^}]*sql\.ErrNoRows[^}]*return nil, nil`, storeCode,
+ "Store.Get must propagate sql.ErrNoRows; callers gating on existence rely on errors.Is(err, sql.ErrNoRows)")
+ assert.Regexp(t, `(?s)func \(s \*Store\) Get\([^)]*\) \([^)]*\) \{[^}]*return nil, err`, storeCode,
+ "Store.Get must surface the underlying error so sql.ErrNoRows reaches callers; a refactor that adds a nested block before the return would silently bypass the NotRegexp above")
+
+ dataSrc, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "data_source.go"))
+ require.NoError(t, err)
+ dataCode := stripGoComments(string(dataSrc))
+
+ assert.Contains(t, dataCode, "errors.Is(err, sql.ErrNoRows)",
+ "data_source.go must detect missing rows via errors.Is(err, sql.ErrNoRows)")
+ assert.NotContains(t, dataCode, "if item == nil {",
+ "data_source.go must not use (item == nil) to detect missing rows; Get returns sql.ErrNoRows instead")
+
+ storeTestSrc, err := os.ReadFile(filepath.Join(outputDir, "internal", "store", "schema_version_test.go"))
+ require.NoError(t, err)
+ assert.Contains(t, string(storeTestSrc), "func TestGet_MissingRowReturnsErrNoRows(",
+ "template-level Get contract test must land in the emitted store package; dropping it would leave the runtime contract uncovered in printed CLIs")
+}
+
// TestGenerateMCPSQLToolUsesReadOnlyStore guards the agent-native security
// model. The MCP sql and search tools advertise readOnlyHint=true to MCP
// hosts so the host auto-approves invocations; a false readOnlyHint on a
diff --git a/internal/generator/templates/data_source.go.tmpl b/internal/generator/templates/data_source.go.tmpl
index b2397b58..9a83d1b7 100644
--- a/internal/generator/templates/data_source.go.tmpl
+++ b/internal/generator/templates/data_source.go.tmpl
@@ -5,7 +5,9 @@ package cli
import (
"context"
+ "database/sql"
"encoding/json"
+ "errors"
"fmt"
"net"
"net/url"
@@ -253,11 +255,11 @@ func resolveLocal(ctx context.Context, resourceType string, isList bool, path st
item, err := db.Get(resourceType, id)
if err != nil {
+ if errors.Is(err, sql.ErrNoRows) {
+ return nil, DataProvenance{}, fmt.Errorf("resource %q with ID %q not found in local store. Run '{{.Name}}-pp-cli sync' first", resourceType, id)
+ }
return nil, DataProvenance{}, fmt.Errorf("querying local store: %w", err)
}
- if item == nil {
- return nil, DataProvenance{}, fmt.Errorf("resource %q with ID %q not found in local store. Run '{{.Name}}-pp-cli sync' first", resourceType, id)
- }
return item, prov, nil
}
diff --git a/internal/generator/templates/store.go.tmpl b/internal/generator/templates/store.go.tmpl
index 9d8efcc6..a022117d 100644
--- a/internal/generator/templates/store.go.tmpl
+++ b/internal/generator/templates/store.go.tmpl
@@ -642,15 +642,14 @@ func (s *Store) Upsert(resourceType, id string, data json.RawMessage) error {
return tx.Commit()
}
+// Propagates sql.ErrNoRows on a miss so callers can distinguish absence from
+// other scan errors via errors.Is.
func (s *Store) Get(resourceType, id string) (json.RawMessage, error) {
var data string
err := s.db.QueryRow(
`SELECT data FROM resources WHERE resource_type = ? AND id = ?`,
resourceType, id,
).Scan(&data)
- if err == sql.ErrNoRows {
- return nil, nil
- }
if err != nil {
return nil, err
}
diff --git a/internal/generator/templates/store_schema_version_test.go.tmpl b/internal/generator/templates/store_schema_version_test.go.tmpl
index 10f54b80..b2234d73 100644
--- a/internal/generator/templates/store_schema_version_test.go.tmpl
+++ b/internal/generator/templates/store_schema_version_test.go.tmpl
@@ -302,6 +302,36 @@ func TestResources_CompositeKeyPreservesOverlappingIDs(t *testing.T) {
}
}
+// Callers detect missing rows via errors.Is(err, sql.ErrNoRows); present
+// rows return the JSON payload with a nil error.
+func TestGet_MissingRowReturnsErrNoRows(t *testing.T) {
+ dbPath := filepath.Join(t.TempDir(), "data.db")
+ s, err := Open(dbPath)
+ if err != nil {
+ t.Fatalf("open: %v", err)
+ }
+ defer s.Close()
+
+ data, err := s.Get("missing_type", "missing_id")
+ if !errors.Is(err, sql.ErrNoRows) {
+ t.Fatalf("Get missing row err = %v, want sql.ErrNoRows", err)
+ }
+ if data != nil {
+ t.Fatalf("Get missing row data = %s, want nil", data)
+ }
+
+ if err := s.Upsert("present_type", "present_id", []byte(`{"ok":true}`)); err != nil {
+ t.Fatalf("upsert: %v", err)
+ }
+ got, err := s.Get("present_type", "present_id")
+ if err != nil {
+ t.Fatalf("Get present row: %v", err)
+ }
+ if string(got) != `{"ok":true}` {
+ t.Fatalf("Get present row data = %s, want {\"ok\":true}", got)
+ }
+}
+
func TestMigrate_ResourcesCompositeKeyUpgrade(t *testing.T) {
dbPath := filepath.Join(t.TempDir(), "data.db")
← 9e604a95 test(cli): pin doctor authConfigured short-circuit for OAuth
·
back to Cli Printing Press
·
fix(cli): gate sync since-param emission per resource (#1036 dc5e8c55 →