← back to Cli Printing Press
feat(cli,skills): five remaining retro WUs from postman-explore — store concurrency, narrative validator, proxy joiner, browser-sniff docs (#440)
d24f60edc2fae62bf963b1ec1465ac73497cc139 · 2026-04-30 09:44:42 -0700 · Trevin Chow
* docs(skills): document direct-API-probe fallback and traffic-analysis version literal
WU-5 (F8): Add Step 2a.0 to browser-sniff-capture.md documenting a
curl-with-Chrome-UA probe to try before launching browser-use when the
target site is WAF-protected. Particularly valuable for sites that
front a discoverable proxy-envelope or whose `/api/*` paths exempt
themselves from the JS challenge gating HTML pages. The replayability
check still applies regardless of probe outcome.
WU-8 (F9): Document that traffic-analysis.json's `version` field is the
literal string "1", not semver. Hand-editors who write "1.0.0" hit the
"unsupported traffic analysis version" parser error.
Both findings come from retro #423 (postman-explore session).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cli): bound store migration concurrency with BEGIN IMMEDIATE + retry
WU-7 (F10): Wrap every step of migrate() — schema-version read, column
backfill, CREATE TABLE statements, schema-version stamp — in a single
BEGIN IMMEDIATE transaction pinned to one connection. modernc.org/sqlite's
busy_timeout does not always cover BEGIN/COMMIT contention at the WAL
write lock, so an explicit retry-on-SQLITE_BUSY loop with exponential
backoff (5ms→100ms, capped at 30s total) sits behind every BEGIN and
COMMIT. backfillColumns and ensureColumn now take the pinned *sql.Conn
so their reads/writes participate in the held transaction instead of
routing through s.db's pool and BUSYing against the writer that holds it.
Adds two regression tests:
- TestGenerateStoreMigrateUsesBeginImmediate (generator-level canary):
asserts the emitted store.go contains s.db.Conn(ctx), BEGIN IMMEDIATE,
and COMMIT. Fast failure on template regression.
- TestMigrate_ConcurrentFreshDB (shipped into every printed CLI's test
suite via store_schema_version_test.go.tmpl): opens 8 concurrent Open()
calls on the same fresh DB path and expects all to succeed.
Without this fix, the 8-goroutine test trips SQLITE_BUSY at every stage
of migrate() — the test parallelism scenario the retro identified.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(skills): validate research.json narrative commands against the CLI tree post-generate
WU-4 (F5): Add a REQUIRED post-generation validation step in Phase 2 that
walks every narrative.quickstart[].command and narrative.recipes[].command
against the just-built CLI binary's Cobra tree via --help. Catches LLM- or
human-authored commands that don't actually resolve — wrong resource name,
endpoint dropped because of a complex body, or placeholder strings the
author forgot to replace. Without the check, broken commands ship straight
to the README Quick Start and the SKILL recipes and break the very first
copy-paste a user attempts.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cli): proxy-envelope buildProxyPath honors paths with existing query string
WU-6 (F6): When a proxy-envelope path already carries a query string
(e.g. /api?op=list), buildProxyPath now appends additional params with
& instead of ?. The old form produced two ? separators, which the
upstream proxy rejects as malformed.
F7 (typed sub-resource discriminator routing in sync) is more involved
(profiler detection + spec extension + sync template) and is filed as
a separate follow-up issue.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(cli): apply /simplify findings to retro-WU bundle
- Drop ticket reference from proxy_envelope_test.go comment per
AGENTS.md ("no dates, incidents, or ticket numbers in code comments").
- Tighten test comments that over-anchored to "in CI" / sibling-file
paths so the WHY is self-contained if the surrounding files move.
- withMigrationLock now passes a single shared deadline to both BEGIN
and COMMIT retries, capping worst-case Open() at migrationLockTimeout
(30s) instead of 2× that. A stuck peer on the WAL write lock no longer
blocks startup for a full minute before failing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(cli,skills): apply ce-code-review safe fixes to retro-WU bundle
Fixes from ce-code-review on the WU-4-through-8 bundle. Deferred items
filed as follow-up issues (#436 ctx propagation, #437 buildProxyPath URL
encoding, #438 fast-path version check, #439 runtime test for
buildProxyPath).
Applied:
- Delete dead setSchemaVersion in store.go.tmpl. The migrate() refactor
inlined the PRAGMA write; the stale function shipped into every
generated CLI's store package with a comment that no longer matched
the code (cross-reviewer-corroborated dead-code finding).
- isSQLiteBusy now also matches SQLITE_LOCKED and "database table is
locked" so table-level shared-cache contention triggers retry instead
of falling through as a fatal error.
- migrate() tail simplified to `return withMigrationLock(...)` — the
prior two-statement form falsely implied post-lock work.
- Surface ROLLBACK errors with a stderr warning instead of silent
swallow. Leaving a pinned conn in an open transaction returned to the
pool would confuse later queries; warning lets operators see when
rollback fails (rare, but worth surfacing).
- ROLLBACK now uses context.Background() so caller-context cancellation
can't strand the connection mid-rollback.
- Strengthen TestGenerateStoreMigrateUsesBeginImmediate canary by
stripping Go comments before substring checks. Without this, a
refactor that removes the actual call sites but leaves explanatory
prose containing `BEGIN IMMEDIATE` / `COMMIT` would still pass the
canary. Also asserts `withMigrationLock(` is called from the migrate
body, which is the load-bearing structural invariant.
- Tighten withMigrationLock comment to describe the real upper bound
(migrationLockTimeout + one trailing backoff + driver busy_timeout)
rather than overstating the cap.
- TestMigrate_ConcurrentFreshDB now calls t.Parallel() and skips under
testing.Short() — the worst-case retry path can take up to
migrationLockTimeout, and slow CI shouldn't be forced to pay it.
- SKILL.md narrative-validator bash recipe now fails loudly when
research.json is missing or malformed, when narrative.quickstart and
recipes are both empty, and when a command's awk-extracted subcommand
words are empty. The prior recipe could pass silently in any of those
cases — defeating the entire purpose of the WU-4 validation step.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Files touched
M internal/generator/generator_test.goA internal/generator/proxy_envelope_test.goM internal/generator/templates/client.go.tmplM internal/generator/templates/store.go.tmplM internal/generator/templates/store_schema_version_test.go.tmplM skills/printing-press/SKILL.mdM skills/printing-press/references/browser-sniff-capture.md
Diff
commit d24f60edc2fae62bf963b1ec1465ac73497cc139
Author: Trevin Chow <trevin@trevinchow.com>
Date: Thu Apr 30 09:44:42 2026 -0700
feat(cli,skills): five remaining retro WUs from postman-explore — store concurrency, narrative validator, proxy joiner, browser-sniff docs (#440)
* docs(skills): document direct-API-probe fallback and traffic-analysis version literal
WU-5 (F8): Add Step 2a.0 to browser-sniff-capture.md documenting a
curl-with-Chrome-UA probe to try before launching browser-use when the
target site is WAF-protected. Particularly valuable for sites that
front a discoverable proxy-envelope or whose `/api/*` paths exempt
themselves from the JS challenge gating HTML pages. The replayability
check still applies regardless of probe outcome.
WU-8 (F9): Document that traffic-analysis.json's `version` field is the
literal string "1", not semver. Hand-editors who write "1.0.0" hit the
"unsupported traffic analysis version" parser error.
Both findings come from retro #423 (postman-explore session).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cli): bound store migration concurrency with BEGIN IMMEDIATE + retry
WU-7 (F10): Wrap every step of migrate() — schema-version read, column
backfill, CREATE TABLE statements, schema-version stamp — in a single
BEGIN IMMEDIATE transaction pinned to one connection. modernc.org/sqlite's
busy_timeout does not always cover BEGIN/COMMIT contention at the WAL
write lock, so an explicit retry-on-SQLITE_BUSY loop with exponential
backoff (5ms→100ms, capped at 30s total) sits behind every BEGIN and
COMMIT. backfillColumns and ensureColumn now take the pinned *sql.Conn
so their reads/writes participate in the held transaction instead of
routing through s.db's pool and BUSYing against the writer that holds it.
Adds two regression tests:
- TestGenerateStoreMigrateUsesBeginImmediate (generator-level canary):
asserts the emitted store.go contains s.db.Conn(ctx), BEGIN IMMEDIATE,
and COMMIT. Fast failure on template regression.
- TestMigrate_ConcurrentFreshDB (shipped into every printed CLI's test
suite via store_schema_version_test.go.tmpl): opens 8 concurrent Open()
calls on the same fresh DB path and expects all to succeed.
Without this fix, the 8-goroutine test trips SQLITE_BUSY at every stage
of migrate() — the test parallelism scenario the retro identified.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(skills): validate research.json narrative commands against the CLI tree post-generate
WU-4 (F5): Add a REQUIRED post-generation validation step in Phase 2 that
walks every narrative.quickstart[].command and narrative.recipes[].command
against the just-built CLI binary's Cobra tree via --help. Catches LLM- or
human-authored commands that don't actually resolve — wrong resource name,
endpoint dropped because of a complex body, or placeholder strings the
author forgot to replace. Without the check, broken commands ship straight
to the README Quick Start and the SKILL recipes and break the very first
copy-paste a user attempts.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cli): proxy-envelope buildProxyPath honors paths with existing query string
WU-6 (F6): When a proxy-envelope path already carries a query string
(e.g. /api?op=list), buildProxyPath now appends additional params with
& instead of ?. The old form produced two ? separators, which the
upstream proxy rejects as malformed.
F7 (typed sub-resource discriminator routing in sync) is more involved
(profiler detection + spec extension + sync template) and is filed as
a separate follow-up issue.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(cli): apply /simplify findings to retro-WU bundle
- Drop ticket reference from proxy_envelope_test.go comment per
AGENTS.md ("no dates, incidents, or ticket numbers in code comments").
- Tighten test comments that over-anchored to "in CI" / sibling-file
paths so the WHY is self-contained if the surrounding files move.
- withMigrationLock now passes a single shared deadline to both BEGIN
and COMMIT retries, capping worst-case Open() at migrationLockTimeout
(30s) instead of 2× that. A stuck peer on the WAL write lock no longer
blocks startup for a full minute before failing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(cli,skills): apply ce-code-review safe fixes to retro-WU bundle
Fixes from ce-code-review on the WU-4-through-8 bundle. Deferred items
filed as follow-up issues (#436 ctx propagation, #437 buildProxyPath URL
encoding, #438 fast-path version check, #439 runtime test for
buildProxyPath).
Applied:
- Delete dead setSchemaVersion in store.go.tmpl. The migrate() refactor
inlined the PRAGMA write; the stale function shipped into every
generated CLI's store package with a comment that no longer matched
the code (cross-reviewer-corroborated dead-code finding).
- isSQLiteBusy now also matches SQLITE_LOCKED and "database table is
locked" so table-level shared-cache contention triggers retry instead
of falling through as a fatal error.
- migrate() tail simplified to `return withMigrationLock(...)` — the
prior two-statement form falsely implied post-lock work.
- Surface ROLLBACK errors with a stderr warning instead of silent
swallow. Leaving a pinned conn in an open transaction returned to the
pool would confuse later queries; warning lets operators see when
rollback fails (rare, but worth surfacing).
- ROLLBACK now uses context.Background() so caller-context cancellation
can't strand the connection mid-rollback.
- Strengthen TestGenerateStoreMigrateUsesBeginImmediate canary by
stripping Go comments before substring checks. Without this, a
refactor that removes the actual call sites but leaves explanatory
prose containing `BEGIN IMMEDIATE` / `COMMIT` would still pass the
canary. Also asserts `withMigrationLock(` is called from the migrate
body, which is the load-bearing structural invariant.
- Tighten withMigrationLock comment to describe the real upper bound
(migrationLockTimeout + one trailing backoff + driver busy_timeout)
rather than overstating the cap.
- TestMigrate_ConcurrentFreshDB now calls t.Parallel() and skips under
testing.Short() — the worst-case retry path can take up to
migrationLockTimeout, and slow CI shouldn't be forced to pay it.
- SKILL.md narrative-validator bash recipe now fails loudly when
research.json is missing or malformed, when narrative.quickstart and
recipes are both empty, and when a command's awk-extracted subcommand
words are empty. The prior recipe could pass silently in any of those
cases — defeating the entire purpose of the WU-4 validation step.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
internal/generator/generator_test.go | 94 +++++++++++
internal/generator/proxy_envelope_test.go | 41 +++++
internal/generator/templates/client.go.tmpl | 10 +-
internal/generator/templates/store.go.tmpl | 179 ++++++++++++++++-----
.../templates/store_schema_version_test.go.tmpl | 37 +++++
skills/printing-press/SKILL.md | 85 ++++++++++
.../references/browser-sniff-capture.md | 37 ++++-
7 files changed, 444 insertions(+), 39 deletions(-)
diff --git a/internal/generator/generator_test.go b/internal/generator/generator_test.go
index 1e0a9749..d19b3a9b 100644
--- a/internal/generator/generator_test.go
+++ b/internal/generator/generator_test.go
@@ -1282,6 +1282,100 @@ func TestGenerateWithEmptyOwner(t *testing.T) {
assert.NotContains(t, string(gomod), "module github.com/")
}
+// TestGenerateStoreMigrateUsesBeginImmediate is a fast canary that the
+// emitted store.go runs migrations inside a BEGIN IMMEDIATE transaction
+// pinned to a single connection. Without it, parallel Open() against a
+// fresh DB races per CREATE TABLE statement and trips SQLITE_BUSY despite
+// the busy_timeout. The runtime concurrency check ships into every
+// generated CLI's store package; this test fails the generator
+// immediately on regression so the slower runtime check doesn't have
+// to be the first signal.
+func TestGenerateStoreMigrateUsesBeginImmediate(t *testing.T) {
+ t.Parallel()
+
+ apiSpec := &spec.APISpec{
+ Name: "begin-immediate-canary",
+ Version: "0.1.0",
+ BaseURL: "https://api.example.com",
+ Auth: spec.AuthConfig{Type: "none"},
+ Config: spec.ConfigSpec{
+ Format: "toml",
+ Path: "~/.config/begin-immediate-canary-pp-cli/config.toml",
+ },
+ Resources: map[string]spec.Resource{
+ "things": {
+ Description: "Things",
+ Endpoints: map[string]spec.Endpoint{
+ "list": {
+ Method: "GET",
+ Path: "/things",
+ Description: "List things",
+ Response: spec.ResponseDef{Type: "array"},
+ Params: []spec.Param{
+ {Name: "id", Type: "string"},
+ {Name: "name", Type: "string"},
+ },
+ },
+ },
+ },
+ },
+ }
+
+ 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)
+ src := string(storeSrc)
+
+ // Stripping comments first prevents false negatives from a refactor that
+ // removes the call sites but leaves explanatory prose containing the same
+ // keywords behind. The check would otherwise pass on comments alone.
+ codeOnly := stripGoComments(src)
+
+ assert.Contains(t, codeOnly, `withMigrationLock(`,
+ "migrate must dispatch the lock helper — without this call, the BEGIN/COMMIT wrapper is unreachable")
+ assert.Contains(t, codeOnly, `s.db.Conn(ctx)`,
+ "migrate must pin a connection — BEGIN/COMMIT pairs must run on the same connection")
+ assert.Contains(t, codeOnly, `BEGIN IMMEDIATE`,
+ "migrate must wrap migrations in BEGIN IMMEDIATE so concurrent fresh-DB Opens serialize on the RESERVED lock instead of racing per-statement")
+ assert.Contains(t, codeOnly, `COMMIT`,
+ "migrate must commit the transaction explicitly")
+}
+
+// stripGoComments removes // line comments and /* ... */ block comments from
+// Go source. Crude but sufficient for canary assertions on emitted templates;
+// it doesn't try to parse string literals (none of the asserted substrings
+// appear in literals in the templates we check).
+func stripGoComments(src string) string {
+ var b strings.Builder
+ b.Grow(len(src))
+ i := 0
+ for i < len(src) {
+ if i+1 < len(src) && src[i] == '/' && src[i+1] == '/' {
+ for i < len(src) && src[i] != '\n' {
+ i++
+ }
+ continue
+ }
+ if i+1 < len(src) && src[i] == '/' && src[i+1] == '*' {
+ i += 2
+ for i+1 < len(src) && (src[i] != '*' || src[i+1] != '/') {
+ i++
+ }
+ if i+1 < len(src) {
+ i += 2
+ }
+ continue
+ }
+ b.WriteByte(src[i])
+ i++
+ }
+ return b.String()
+}
+
func TestGenerateStoreWithBatchResourceDoesNotDuplicateUpsertBatch(t *testing.T) {
t.Parallel()
diff --git a/internal/generator/proxy_envelope_test.go b/internal/generator/proxy_envelope_test.go
new file mode 100644
index 00000000..05952fcf
--- /dev/null
+++ b/internal/generator/proxy_envelope_test.go
@@ -0,0 +1,41 @@
+package generator
+
+import (
+ "os"
+ "path/filepath"
+ "testing"
+
+ "github.com/stretchr/testify/require"
+)
+
+// TestProxyEnvelopeBuildPathHonorsExistingQueryString — when a proxy-
+// envelope path already carries a query string (e.g. /api?op=list),
+// buildProxyPath must use `&` to append additional params instead of `?`.
+// Two `?` separators in one URL produce a path the upstream proxy rejects
+// as malformed.
+//
+// We assert against the emitted client.go rather than calling the helper
+// directly because buildProxyPath only exists inside the proxy-envelope
+// template branch.
+func TestProxyEnvelopeBuildPathHonorsExistingQueryString(t *testing.T) {
+ t.Parallel()
+
+ apiSpec := minimalSpec("proxy-joiner")
+ apiSpec.ClientPattern = "proxy-envelope"
+
+ outputDir := filepath.Join(t.TempDir(), "proxy-joiner-pp-cli")
+ require.NoError(t, New(apiSpec, outputDir).Generate())
+
+ clientSrc, err := os.ReadFile(filepath.Join(outputDir, "internal", "client", "client.go"))
+ require.NoError(t, err)
+ src := string(clientSrc)
+
+ require.Contains(t, src, "func buildProxyPath(",
+ "proxy-envelope client must emit buildProxyPath helper")
+ require.Contains(t, src, `joiner := "?"`,
+ "buildProxyPath must default to ? joiner")
+ require.Contains(t, src, `strings.Contains(path, "?")`,
+ "buildProxyPath must check for an existing query string before appending")
+ require.Contains(t, src, `joiner = "&"`,
+ "buildProxyPath must use & when path already contains ?")
+}
diff --git a/internal/generator/templates/client.go.tmpl b/internal/generator/templates/client.go.tmpl
index ff65bcea..7cb80011 100644
--- a/internal/generator/templates/client.go.tmpl
+++ b/internal/generator/templates/client.go.tmpl
@@ -72,6 +72,10 @@ func serviceForPath(path string) string {
}
// buildProxyPath appends query params into the path string for the envelope.
+// When the spec-declared path already carries a query string (e.g. an
+// /api?op=list endpoint that takes additional params), use `&` as the
+// joiner so the result stays parseable instead of producing two `?`
+// separators.
func buildProxyPath(path string, params map[string]string) string {
if len(params) == 0 {
return path
@@ -85,7 +89,11 @@ func buildProxyPath(path string, params map[string]string) string {
if len(parts) == 0 {
return path
}
- return path + "?" + strings.Join(parts, "&")
+ joiner := "?"
+ if strings.Contains(path, "?") {
+ joiner = "&"
+ }
+ return path + joiner + strings.Join(parts, "&")
}
{{end}}
diff --git a/internal/generator/templates/store.go.tmpl b/internal/generator/templates/store.go.tmpl
index d9f94013..d4fc9bbb 100644
--- a/internal/generator/templates/store.go.tmpl
+++ b/internal/generator/templates/store.go.tmpl
@@ -7,6 +7,7 @@
package store
import (
+ "context"
"database/sql"
"encoding/json"
"fmt"
@@ -90,18 +91,6 @@ func (s *Store) SchemaVersion() (int, error) {
return v, nil
}
-// setSchemaVersion stamps PRAGMA user_version. The value is not parameterizable
-// in SQLite's PRAGMA syntax, so it is formatted into the statement; migrate()
-// only calls this with the compile-time StoreSchemaVersion constant, so there
-// is no untrusted input.
-func (s *Store) setSchemaVersion(version int) error {
- stmt := fmt.Sprintf(`PRAGMA user_version = %d`, version)
- if _, err := s.db.Exec(stmt); err != nil {
- return fmt.Errorf("stamp user_version: %w", err)
- }
- return nil
-}
-
// ensureColumn adds a column to an existing table if it isn't already
// present. It is the upgrade-path safety valve for schema additions:
// CREATE TABLE IF NOT EXISTS is a no-op when the table already exists, so
@@ -111,10 +100,13 @@ func (s *Store) setSchemaVersion(version int) error {
//
// Skips silently if the table doesn't yet exist (fresh install — the
// CREATE TABLE migration will create it with the column already declared)
-// or if the column already exists.
-func (s *Store) ensureColumn(table, column, decl string) error {
+// or if the column already exists. Runs on the pinned migration
+// connection so it sees the writes performed by the in-flight BEGIN
+// IMMEDIATE transaction; using s.db here would route through the pool
+// and BUSY against the holding writer under concurrent migrators.
+func (s *Store) ensureColumn(ctx context.Context, conn *sql.Conn, table, column, decl string) error {
var name string
- err := s.db.QueryRow(
+ err := conn.QueryRowContext(ctx,
`SELECT name FROM sqlite_master WHERE type='table' AND name=?`, table,
).Scan(&name)
if err == sql.ErrNoRows {
@@ -124,7 +116,7 @@ func (s *Store) ensureColumn(table, column, decl string) error {
return fmt.Errorf("checking table %s: %w", table, err)
}
- rows, err := s.db.Query(fmt.Sprintf(`PRAGMA table_info("%s")`, table))
+ rows, err := conn.QueryContext(ctx, fmt.Sprintf(`PRAGMA table_info("%s")`, table))
if err != nil {
return fmt.Errorf("table_info %s: %w", table, err)
}
@@ -145,7 +137,7 @@ func (s *Store) ensureColumn(table, column, decl string) error {
return fmt.Errorf("iterating table_info %s: %w", table, err)
}
- if _, err := s.db.Exec(fmt.Sprintf(`ALTER TABLE "%s" ADD COLUMN "%s" %s`, table, column, decl)); err != nil {
+ if _, err := conn.ExecContext(ctx, fmt.Sprintf(`ALTER TABLE "%s" ADD COLUMN "%s" %s`, table, column, decl)); err != nil {
// A concurrent Open() may have added the column between our
// PRAGMA check and this ALTER. SQLite returns SQLITE_ERROR with
// "duplicate column name", which busy_timeout does not retry.
@@ -172,7 +164,7 @@ func (s *Store) ensureColumn(table, column, decl string) error {
// double-quote characters into the Go string and break compilation for
// any spec whose dependent-resource snake_cased name is a SQL reserved
// word.
-func (s *Store) backfillColumns() error {
+func (s *Store) backfillColumns(ctx context.Context, conn *sql.Conn) error {
for _, c := range []struct{ table, column, decl string }{
{{- range $table := .Tables}}
{{- range $col := $table.Columns}}
@@ -182,7 +174,7 @@ func (s *Store) backfillColumns() error {
{{- end}}
{{- end}}
} {
- if err := s.ensureColumn(c.table, c.column, c.decl); err != nil {
+ if err := s.ensureColumn(ctx, conn, c.table, c.column, c.decl); err != nil {
return err
}
}
@@ -190,17 +182,12 @@ func (s *Store) backfillColumns() error {
}
func (s *Store) migrate() error {
- current, err := s.SchemaVersion()
+ ctx := context.Background()
+ conn, err := s.db.Conn(ctx)
if err != nil {
- return fmt.Errorf("reading schema version: %w", err)
- }
- if current > StoreSchemaVersion {
- return fmt.Errorf("database schema version %d is newer than supported version %d; upgrade the CLI binary or open an older database", current, StoreSchemaVersion)
- }
-
- if err := s.backfillColumns(); err != nil {
- return fmt.Errorf("backfilling columns: %w", err)
+ return fmt.Errorf("acquiring migration connection: %w", err)
}
+ defer conn.Close()
migrations := []string{
`CREATE TABLE IF NOT EXISTS resources (
@@ -270,22 +257,140 @@ func (s *Store) migrate() error {
{{- end}}
}
- for _, m := range migrations {
- if _, err := s.db.Exec(m); err != nil {
- return fmt.Errorf("migration failed: %w", err)
+ // Run every migration — including the column backfill and the
+ // schema-version stamp — inside a single BEGIN IMMEDIATE transaction
+ // pinned to one connection. IMMEDIATE acquires SQLite's RESERVED lock
+ // at BEGIN time so concurrent migrators serialize on it instead of
+ // racing per-statement and tripping SQLITE_BUSY despite busy_timeout.
+ // modernc.org/sqlite's busy_timeout does not always cover write-write
+ // contention at BEGIN/COMMIT time, so we retry both explicitly on
+ // SQLITE_BUSY for up to migrationLockTimeout.
+ return withMigrationLock(ctx, conn, func() error {
+ // Reading user_version inside the lock avoids racing against
+ // another fresh-DB initializer that hasn't yet stamped the
+ // version (which would manifest as a transient SQLITE_BUSY).
+ var current int
+ if err := conn.QueryRowContext(ctx, `PRAGMA user_version`).Scan(¤t); err != nil {
+ return fmt.Errorf("reading schema version: %w", err)
+ }
+ if current > StoreSchemaVersion {
+ return fmt.Errorf("database schema version %d is newer than supported version %d; upgrade the CLI binary or open an older database", current, StoreSchemaVersion)
+ }
+
+ if err := s.backfillColumns(ctx, conn); err != nil {
+ return fmt.Errorf("backfilling columns: %w", err)
+ }
+ for _, m := range migrations {
+ if _, err := conn.ExecContext(ctx, m); err != nil {
+ return fmt.Errorf("migration failed: %w", err)
+ }
+ }
+ // Stamp the schema version. On a fresh DB this writes 1; on an
+ // already-stamped DB this is a no-op write of the same value.
+ // An older DB with user_version = 0 and pre-existing tables
+ // gets stamped here without any data rewrites because the
+ // migrations above are idempotent via CREATE TABLE IF NOT EXISTS.
+ if _, err := conn.ExecContext(ctx, fmt.Sprintf(`PRAGMA user_version = %d`, StoreSchemaVersion)); err != nil {
+ return fmt.Errorf("stamp user_version: %w", err)
+ }
+ return nil
+ })
+}
+
+const (
+ migrationLockTimeout = 30 * time.Second
+ migrationLockBackoffMin = 5 * time.Millisecond
+ migrationLockBackoffMax = 100 * time.Millisecond
+)
+
+// withMigrationLock runs fn inside a BEGIN IMMEDIATE / COMMIT pair on
+// conn, retrying both BEGIN and COMMIT on SQLITE_BUSY against a single
+// shared deadline so a stuck peer caps Open() latency at roughly
+// migrationLockTimeout — not 2× that, as a per-call deadline would. The
+// real upper bound is migrationLockTimeout plus one trailing backoff
+// interval (≤100ms) plus the driver's busy_timeout for the in-flight
+// Exec, since the deadline is checked after each failed attempt rather
+// than as a hard wall-clock cutoff. The body itself is responsible for
+// using conn (not s.db) so the writes participate in the held
+// transaction.
+func withMigrationLock(ctx context.Context, conn *sql.Conn, fn func() error) error {
+ deadline := time.Now().Add(migrationLockTimeout)
+ if err := execWithBusyRetry(ctx, conn, "BEGIN IMMEDIATE", "begin migration transaction", deadline); err != nil {
+ return err
+ }
+ committed := false
+ defer func() {
+ if committed {
+ return
}
+ // ROLLBACK uses context.Background() so caller-context cancellation
+ // can't strand the connection in an open transaction. A failed
+ // rollback is rare on local SQLite (broken file handle, fatal
+ // driver error) but worth surfacing — silent swallow leaves a
+ // pinned connection returned to the pool with state that will
+ // confuse later queries.
+ if _, rerr := conn.ExecContext(context.Background(), "ROLLBACK"); rerr != nil {
+ fmt.Fprintf(os.Stderr, "warning: store migration rollback failed: %v\n", rerr)
+ }
+ }()
+
+ if err := fn(); err != nil {
+ return err
}
- // Stamp the schema version after successful migration. On a fresh DB
- // this writes 1; on an already-stamped DB this is a no-op write of the
- // same value. An older DB with user_version = 0 and pre-existing tables
- // gets stamped here without any data rewrites because the migrations
- // above are idempotent via CREATE TABLE IF NOT EXISTS.
- if err := s.setSchemaVersion(StoreSchemaVersion); err != nil {
+
+ if err := execWithBusyRetry(ctx, conn, "COMMIT", "commit migration transaction", deadline); err != nil {
return err
}
+ committed = true
return nil
}
+// execWithBusyRetry runs stmt on conn and retries on SQLITE_BUSY until
+// deadline. It covers both BEGIN IMMEDIATE and COMMIT contention;
+// modernc.org/sqlite's busy_timeout does not reliably cover either when
+// multiple connections race for the WAL write lock.
+func execWithBusyRetry(ctx context.Context, conn *sql.Conn, stmt, label string, deadline time.Time) error {
+ backoff := migrationLockBackoffMin
+ for {
+ _, err := conn.ExecContext(ctx, stmt)
+ if err == nil {
+ return nil
+ }
+ if !isSQLiteBusy(err) {
+ return fmt.Errorf("%s: %w", label, err)
+ }
+ if time.Now().After(deadline) {
+ return fmt.Errorf("%s: timed out after %s waiting for write lock: %w", label, migrationLockTimeout, err)
+ }
+ select {
+ case <-ctx.Done():
+ return fmt.Errorf("%s: %w", label, ctx.Err())
+ case <-time.After(backoff):
+ }
+ if backoff *= 2; backoff > migrationLockBackoffMax {
+ backoff = migrationLockBackoffMax
+ }
+ }
+}
+
+// isSQLiteBusy reports whether err is a retryable SQLite lock condition.
+// Covers both the file-level WAL writer race (SQLITE_BUSY / "database is
+// locked") and the table-level shared-cache contention (SQLITE_LOCKED /
+// "database table is locked"). The match is on the error string because
+// modernc.org/sqlite does not export an error type the generated code
+// can switch on without dragging the driver package into every store
+// consumer.
+func isSQLiteBusy(err error) bool {
+ if err == nil {
+ return false
+ }
+ msg := err.Error()
+ return strings.Contains(msg, "SQLITE_BUSY") ||
+ strings.Contains(msg, "SQLITE_LOCKED") ||
+ strings.Contains(msg, "database is locked") ||
+ strings.Contains(msg, "database table is locked")
+}
+
func (s *Store) upsertGenericResourceTx(tx *sql.Tx, resourceType, id string, data json.RawMessage) error {
_, err := tx.Exec(
`INSERT INTO resources (id, resource_type, data, synced_at, updated_at)
diff --git a/internal/generator/templates/store_schema_version_test.go.tmpl b/internal/generator/templates/store_schema_version_test.go.tmpl
index 9f44969d..acac455d 100644
--- a/internal/generator/templates/store_schema_version_test.go.tmpl
+++ b/internal/generator/templates/store_schema_version_test.go.tmpl
@@ -6,6 +6,7 @@ package store
import (
"database/sql"
"path/filepath"
+ "sync"
"testing"
_ "modernc.org/sqlite"
@@ -88,6 +89,42 @@ func TestSchemaVersion_RefusesNewerDB(t *testing.T) {
}
}
+// TestMigrate_ConcurrentFreshDB exercises the BEGIN IMMEDIATE migration
+// transaction. Without it, N goroutines opening the same fresh DB in
+// parallel race per CREATE TABLE statement and trip SQLITE_BUSY despite
+// the busy_timeout. With it, they serialize on the RESERVED lock
+// acquired at BEGIN time and every Open succeeds.
+func TestMigrate_ConcurrentFreshDB(t *testing.T) {
+ if testing.Short() {
+ t.Skip("concurrent migration test can take up to migrationLockTimeout under contention")
+ }
+ t.Parallel()
+
+ dbPath := filepath.Join(t.TempDir(), "data.db")
+
+ const n = 8
+ errs := make(chan error, n)
+ var wg sync.WaitGroup
+ wg.Add(n)
+ for i := 0; i < n; i++ {
+ go func() {
+ defer wg.Done()
+ s, err := Open(dbPath)
+ if err != nil {
+ errs <- err
+ return
+ }
+ s.Close()
+ }()
+ }
+ wg.Wait()
+ close(errs)
+
+ for err := range errs {
+ t.Fatalf("concurrent Open failed: %v", err)
+ }
+}
+
// TestSchemaVersion_ReopenIsIdempotent verifies that opening an already
// correctly-stamped DB is a no-op — the second open reads the version
// and the migrations are all idempotent (CREATE TABLE IF NOT EXISTS).
diff --git a/skills/printing-press/SKILL.md b/skills/printing-press/SKILL.md
index b4e997ea..d88d548f 100644
--- a/skills/printing-press/SKILL.md
+++ b/skills/printing-press/SKILL.md
@@ -1663,6 +1663,91 @@ Phase 1 research brief for auth requirements and manually add env var support to
`config.go` using the pattern: add `APIKey`/`APIKeySource` fields to the Config struct,
and `os.Getenv("<API>_API_KEY")` in the Load function.
+**REQUIRED: Validate narrative `command` strings resolve in the CLI tree.**
+The LLM (or human) authoring `research.json` can name commands that don't actually
+exist in the generated CLI — `<cli> stats` when the real shape is `<cli> reports stats`,
+or a command that was dropped because its endpoint had a complex body. Without a check,
+the broken commands ship to the README's Quick Start (`narrative.quickstart`) and the
+SKILL's recipes (`narrative.recipes`); users copy-paste them and hit `unknown command`
+on the very first invocation.
+
+Build the CLI binary first (the CLI does not need to run against a live API for this
+check; `--help` is offline). Then for each `narrative.quickstart[].command` and
+`narrative.recipes[].command`, strip the binary name and trailing arguments to get the
+command path, and confirm it walks the Cobra tree:
+
+```bash
+QUICKSTART_BINARY="$CLI_WORK_DIR/<api>-pp-cli"
+go build -o "$QUICKSTART_BINARY" "$CLI_WORK_DIR/cmd/<api>-pp-cli"
+
+# Fail loudly if research.json is missing or malformed — silent jq output
+# would otherwise pass an empty pipeline through the loop and falsely
+# report "everything looks fine" when nothing was actually checked.
+if [ ! -f "$API_RUN_DIR/research.json" ]; then
+ echo "ERROR: $API_RUN_DIR/research.json not found; cannot validate narrative commands" >&2
+ exit 1
+fi
+if ! jq empty "$API_RUN_DIR/research.json" >/dev/null 2>&1; then
+ echo "ERROR: $API_RUN_DIR/research.json is not valid JSON" >&2
+ exit 1
+fi
+
+# Track whether any narrative command was actually walked. An empty quickstart
+# AND empty recipes list is itself worth flagging — the LLM authoring the
+# research.json may have omitted both sections by mistake.
+walked=0
+missing=0
+
+while IFS=$'\t' read -r section cmd; do
+ # Drop the leading binary name and any --flag/positional arg suffix.
+ # Keep only the literal subcommand words (everything before the first
+ # word that starts with `-` or that contains `=`/`:`/non-alphanumerics).
+ words=$(printf '%s\n' "$cmd" \
+ | awk '{ for (i=2; i<=NF; i++) { if ($i ~ /^-/ || $i ~ /[^a-zA-Z0-9_-]/) break; printf "%s ", $i } }')
+ # Strip trailing whitespace; awk's "%s " emits a trailing space.
+ words=$(printf '%s' "$words" | sed 's/[[:space:]]*$//')
+ if [ -z "$words" ]; then
+ # Bare-binary or pure-flag commands ("<cli>", "<cli> --version") have
+ # nothing for `--help` to validate. Flag instead of silently passing.
+ echo "EMPTY [$section]: $cmd has no subcommand words to verify" >&2
+ missing=$((missing + 1))
+ continue
+ fi
+ walked=$((walked + 1))
+ # shellcheck disable=SC2086 # words is a deliberate splat into argv
+ if ! "$QUICKSTART_BINARY" $words --help >/dev/null 2>&1; then
+ echo "MISSING [$section]: $cmd → $words" >&2
+ missing=$((missing + 1))
+ fi
+done < <(jq -r '
+ ((.narrative.quickstart // []) | .[] | "quickstart\t" + .command),
+ ((.narrative.recipes // []) | .[] | "recipes\t" + .command)
+' "$API_RUN_DIR/research.json")
+
+if [ "$walked" -eq 0 ] && [ "$missing" -eq 0 ]; then
+ echo "WARNING: research.json has no narrative.quickstart or narrative.recipes entries" >&2
+fi
+if [ "$missing" -gt 0 ]; then
+ echo "ERROR: $missing narrative command(s) failed validation; fix research.json before continuing" >&2
+ exit 1
+fi
+```
+
+If any commands are reported missing, fix them in `research.json` before continuing.
+Common causes:
+
+- Resource was renamed during generation (typically the spec uses `users` but the LLM
+ wrote `user` in research.json).
+- The endpoint exists but is hidden (had a complex body and was dropped from the
+ promoted-command surface; reach it via the typed `<resource> <endpoint>` form).
+- The command name is a placeholder (`<cli> example`) that should have been replaced
+ with a real path.
+
+`narrative.quickstart` drives the README Quick Start and `narrative.recipes` drives
+the SKILL.md recipes; getting either wrong silently ships copy-paste-broken examples
+to users. The `--help`-walk check is the cheapest catch and runs offline against the
+just-built binary — no live API access needed.
+
After the description rewrite, update the lock heartbeat:
```bash
diff --git a/skills/printing-press/references/browser-sniff-capture.md b/skills/printing-press/references/browser-sniff-capture.md
index 73142825..52f88646 100644
--- a/skills/printing-press/references/browser-sniff-capture.md
+++ b/skills/printing-press/references/browser-sniff-capture.md
@@ -301,6 +301,38 @@ Do NOT silently proceed without auth when the session has expired. The authentic
If cookies are verified, proceed to Steps 2a/2b capture flow with the authenticated session loaded. The session state file is stored at `$DISCOVERY_DIR/session-state.json`.
+#### Step 2a.0: Direct-API-probe fallback (try before browser-use when WAF-protected)
+
+When `probe-reachability` returns `mode: browser_http` (Cloudflare, Vercel, or another WAF challenge in front of the site), browser-use can still get blocked at runtime even though `surf` would clear the challenge. Before launching browser-use, attempt a direct curl probe with a Chrome User-Agent to see whether the site's API surface answers without a resident browser.
+
+This is most likely to succeed against:
+
+- Sites with a discoverable proxy-envelope endpoint (a single internal route such as `/_api/ws/proxy`, `/api/graphql`, or `/internal/rpc` that fronts the public API).
+- Sites whose public API responds to a Chrome UA without JS challenge (the WAF gates HTML pages but exempts `/api/*` paths).
+
+Probe pattern:
+
+```bash
+# Pick a candidate endpoint surfaced by the Phase 1 research brief or
+# the proxy-envelope detection step above.
+PROBE_URL='https://<site>/<candidate-api-path>'
+
+curl -s -o /tmp/probe-response.json -w '%{http_code}\n' \
+ -A 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/131.0.0.0 Safari/537.36' \
+ -H 'Accept: application/json' \
+ "$PROBE_URL"
+```
+
+Decision criteria:
+
+- **HTTP 200 with structured JSON** — the direct probe is viable. Capture a handful of representative endpoint responses to `$DISCOVERY_DIR/direct-probe-*.json`, then proceed to Step 2a or fall through to Step 2b (manual HAR via DevTools) for the structured browser-sniff capture. **Do not skip the structured capture** — `printing-press browser-sniff` needs a HAR or enriched-capture JSON, not loose curl responses, and the replayability check still has to run against the captured envelope.
+- **HTTP 403/429 with a Cloudflare challenge body** (`<title>Just a moment...</title>`, `cf_chl_opt`, Vercel/Akamai equivalents) — the WAF is blocking direct probes. Fall through to browser-use; the captured surface will need Surf transport and possibly a clearance-cookie step.
+- **HTTP 401/403 with a structured auth error** (`{"error":"unauthenticated"}`, `WWW-Authenticate: Bearer`) — direct probing works but the path is auth-only. Document the path in the brief and route to the authenticated-flow capture.
+
+The replayability check still applies regardless of probe outcome: any endpoint discovered through direct probing must round-trip through `surf` with the same Chrome TLS fingerprint the printed CLI will ship, or the captured URLs are unusable in production.
+
+Example: a public read-only catalog site fronted by Cloudflare exposes `/_api/ws/proxy` (the internal proxy-envelope) and answers a Chrome-UA `POST` with the right service/method/path body. The direct probe confirmed the envelope shape and request fields in seconds; the structured browser-sniff still ran via DevTools HAR to capture the full set of paths the proxy fronts.
+
#### Step 2a: browser-use CLI capture (preferred)
Claude drives browser-use directly via CLI commands — no LLM key needed, no Python API versioning issues. Uses the browser's native Performance API to collect API endpoint URLs from each page.
@@ -749,7 +781,10 @@ If hand-writing or repairing `$DISCOVERY_DIR/traffic-analysis.json`, inspect the
printing-press schema traffic-analysis > "$DISCOVERY_DIR/traffic-analysis.schema.json"
```
-Notably, confidence fields are numbers from `0` to `1`, not strings such as `"high"`.
+Two fields trip up hand-edits often enough to call out:
+
+- **`version`** is the literal string `"1"` — not semver, not `"1.0.0"`. The downstream parser rejects any other value with `unsupported traffic analysis version`.
+- **Confidence fields** are numbers from `0` to `1`, not strings such as `"high"`.
#### Step 4: Report and update spec source
← 1556ff1b fix(cli): emit StringVar for cursor/page/timestamp paginatio
·
back to Cli Printing Press
·
feat(cli): WU-2 sync correctness pass (pagination, ID extrac 9a135062 →