fix: address 10 confirmed review findings from the 0.6.17-0.6.20 range

CHANGELOG.md

@@ -88,6 +88,57 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   a local-only pre-tag gate; this is just an opt-in fail-fast.
 - `package.json` `package-lock.json` and `server.json` all bumped to 0.6.20.
 
+## [0.6.19] - 2026-06-02
+
+Release-flow hardening; no library behavior changes shipped in this version.
+
+### Fixed
+- `release.sh` refuses to push when origin already has `v<version>` pointing
+  at a different commit (rewound tag elsewhere, parallel release race) --
+  previously `git push --follow-tags` silently skipped the stale tag and the
+  GitHub release linked the wrong commit. Compares tag-object SHAs so resume
+  runs don't false-abort.
+- README "Add to Yaw MCP" badge points at the https forwarder so it renders
+  as a link on github.com (raw `yaw://` hrefs are stripped).
+
+### Added
+- `SKIP_LINT=1` escape hatch in `release.sh` for hosts where the npm
+  run-script wrapper segfaults on exit-cleanup (MINGW64-ARM64).
+- `wrapToolHandler` extracted from `index.ts` for testability, with unit
+  coverage of the MCP result wrapper and the connect-failure path (expanded
+  further in 0.6.20).
+
+## [0.6.18] - 2026-05-28
+
+### Changed
+- Release publishing consolidated into `release.sh`: the MCP Registry publish
+  moved into the script and `release.yml` (plus the non-release CI workflows)
+  was dropped. The script hands off to CI when a CI publish path exists and
+  publishes from the workstation otherwise.
+
+### Fixed
+- `release.sh` syncs `server.json` unconditionally, not only inside the bump
+  branch, so a resume run no longer asks mcp-publisher to re-publish the
+  previous version (400 duplicate-version).
+- `release.sh` falls back to the gh CLI session token when
+  `MCP_REGISTRY_TOKEN` is unset.
+- The release confirmation prompt is tty-gated so non-interactive runs don't
+  hang on `read`.
+
+### Docs
+- README install badge swapped to the "Add to Yaw MCP" deep link; `npx`
+  spawn examples pinned to `@latest` for auto-update.
+
+## [0.6.17] - 2026-05-19
+
+### Added
+- `release.sh` accepts an optional pre-release commit message as a second
+  argument: runs the pre-commit checklist, commits tracked changes, then
+  proceeds with the release.
+- Post-publish smoke script (`scripts/post-publish-smoke.sh`) wired into the
+  release flow -- exercises the published tarball via a real `npx` install
+  instead of trusting `npm view` registry metadata.
+
 ## [0.6.16] - 2026-05-18
 
 ### Tests

release.sh

@@ -30,14 +30,20 @@ NC='\033[0m'
 # corrupted bytes then get copy-pasted into bug reports.
 step() { echo -e "\n${CYAN}=== [$1/$TOTAL_STEPS] $2 ===${NC}"; }
 info() { echo -e "${GREEN}  [OK] $1${NC}"; }
-warn() { echo -e "${YELLOW}  [!] $1${NC}"; }
+# warn() writes to stderr so warnings survive stdout redirects -- e.g. the
+# pre-commit path runs `npm run lint:fix >/dev/null`, which would otherwise
+# swallow the SKIP_LINT noop notice and silently skip the formatting gate.
+warn() { echo -e "${YELLOW}  [!] $1${NC}" >&2; }
 fail() { echo -e "${RED}  [X] $1${NC}"; exit 1; }
 
 # SKIP_LINT=1 escape hatch -- wraps `npm`/`pnpm` so lint-related runs are
 # no-ops. Workaround for the MINGW64-ARM64 npm-run-script wrapper that
-# segfaults on exit-cleanup (platform-windows.md). Apply only when the
-# lint runner is broken on the host; CI catches lint regressions anyway.
+# segfaults on exit-cleanup (platform-windows.md). NOTE: this repo has no CI
+# lint gate (release.yml was dropped when registry publish moved into this
+# script), so with SKIP_LINT=1 lint runs NOWHERE for this release -- run
+# `npx biome check src/` on a working runner before tagging.
 if [ "${SKIP_LINT:-}" = "1" ]; then
+  warn "SKIP_LINT=1 -- lint will not run anywhere this release (no CI lint gate exists); run 'npx biome check src/' on a working runner before tagging"
   npm() {
     if [ "$1" = "run" ] && [[ "$2" == lint* ]]; then
       warn "SKIP_LINT=1 -- noop 'npm run $2'"
@@ -143,8 +149,20 @@ if [ "$IS_CI" != "true" ] && [ "$RESUMING" != "true" ]; then
       echo "Aborted."
       exit 0
     fi
+  elif [ "${RELEASE_YES:-}" = "1" ]; then
+    info "RELEASE_YES=1 -- proceeding without confirmation"
   else
-    info "Non-interactive shell -- proceeding without confirmation"
+    # stdin is not a tty: honor a piped reply (`echo y |` proceeds, `echo n |`
+    # aborts) instead of releasing unconditionally -- a piped decline used to
+    # be read and honored before the tty-gate. -t 5 bounds the wait so an
+    # open-but-silent pipe can't hang the release.
+    REPLY=""
+    IFS= read -r -t 5 -n 1 REPLY || true
+    if [[ $REPLY =~ ^[Yy]$ ]]; then
+      info "Non-interactive stdin -- proceeding on piped 'y'"
+    else
+      fail "Non-interactive stdin and no 'y' confirmation -- pipe 'y' or set RELEASE_YES=1 to release non-interactively."
+    fi
   fi
 fi
 
@@ -260,7 +278,12 @@ else
   # reports success, but origin's tag stays at the old SHA -- and the later
   # `gh release create` step then creates a GitHub release linked to that
   # stale commit while npm carries the new one.
-  ORIGIN_TAG_SHA=$(git ls-remote --tags origin "refs/tags/v${VERSION}" 2>/dev/null | awk '{print $1}')
+  # No stderr suppression and an explicit failure branch: under `set -euo
+  # pipefail` a failing ls-remote (network blip, auth) inside the command
+  # substitution would otherwise kill the script with only the generic
+  # ERR-trap line and git's actual error discarded.
+  ORIGIN_TAG_SHA=$(git ls-remote --tags origin "refs/tags/v${VERSION}" | awk '{print $1}') \
+    || fail "Could not query origin for tag v${VERSION} (network or auth failure -- see git's error above). Resolve and re-run."
   if [ -n "$ORIGIN_TAG_SHA" ]; then
     LOCAL_TAG_SHA=$(git rev-parse "v${VERSION}")
     if [ "$ORIGIN_TAG_SHA" != "$LOCAL_TAG_SHA" ]; then
@@ -441,12 +464,25 @@ else
   # io.github.YawLabs/* namespace.
   # Fall back to gh CLI's session token if MCP_REGISTRY_TOKEN is unset --
   # gh auth login (admin:org or read:org scope) covers the namespace claim.
-  : "${MCP_REGISTRY_TOKEN:=$(gh auth token 2>/dev/null || true)}"
+  # Track which source supplied the token so a login failure names the thing
+  # the operator actually controls instead of an env var they never set.
+  TOKEN_SOURCE="env"
+  if [ -z "${MCP_REGISTRY_TOKEN:-}" ]; then
+    MCP_REGISTRY_TOKEN="$(gh auth token 2>/dev/null || true)"
+    TOKEN_SOURCE="gh-cli"
+  fi
   if [ -z "${MCP_REGISTRY_TOKEN:-}" ]; then
-    fail "MCP_REGISTRY_TOKEN unset -- set it to a GitHub PAT with read:org for YawLabs (or run '$MP login github' once interactively to cache the session)."
+    fail "MCP_REGISTRY_TOKEN unset and 'gh auth token' returned nothing -- set a GitHub PAT with read:org for YawLabs, or 'gh auth login' first (or run '$MP login github' once interactively to cache the session)."
+  fi
+  # stdout stays quiet; stderr passes through so mcp-publisher's own
+  # diagnostic isn't hidden behind the generic fail message.
+  if ! "$MP" login github -token "$MCP_REGISTRY_TOKEN" >/dev/null; then
+    if [ "$TOKEN_SOURCE" = "gh-cli" ]; then
+      fail "mcp-publisher login failed using the gh CLI session token (MCP_REGISTRY_TOKEN was unset). The gh token likely lacks read:org for YawLabs -- run 'gh auth refresh -h github.com -s read:org' and re-run."
+    else
+      fail "mcp-publisher login failed -- check MCP_REGISTRY_TOKEN scopes (needs read:org for YawLabs)."
+    fi
   fi
-  "$MP" login github -token "$MCP_REGISTRY_TOKEN" >/dev/null 2>&1 \
-    || fail "mcp-publisher login failed -- check MCP_REGISTRY_TOKEN scopes (needs read:org for YawLabs)"
   "$MP" publish \
     || fail "mcp-publisher publish failed -- npm + GitHub release succeeded, but the MCP Registry did not. Retry the step (re-run the script) once the cause is identified."
   info "Published to MCP Registry"

src/api.ts

@@ -106,10 +106,12 @@ export function getSslConfig(): { rejectUnauthorized: boolean } | undefined {
  * until `shutdown()` runs and a subsequent call rebuilds the pool.
  *
  * Two values are intentionally re-read on every request and are NOT snapshotted
- * here: `getMaxRows()` (so a one-off bump to `POSTGRES_MAX_ROWS` takes effect
- * immediately, no restart) and `isWritesAllowed()` (so flipping `ALLOW_WRITES`
- * on or off between tool calls is observed without a process restart -- the
- * operator uses this when they want to briefly run a write, then flip back).
+ * here: `getMaxRows()` and `isWritesAllowed()`. The per-request re-read matters
+ * for in-process callers (tests set `process.env.ALLOW_WRITES` between calls)
+ * and keeps the flag out of the pool snapshot above. It does NOT give an
+ * operator a live toggle: a stdio MCP server's environment is fixed at spawn
+ * by the host's config, so changing `ALLOW_WRITES` (or `POSTGRES_MAX_ROWS`)
+ * on a running server requires the MCP host to restart it.
  */
 export function getPool(): pg.Pool {
   if (pool) return pool;
@@ -152,10 +154,13 @@ export interface QueryResult {
 // but is never read back (no result row references the dead oid), so it's
 // wasted memory, not a correctness bug.
 //
-// Concurrency: the bootstrap read is unsynchronized, so with POSTGRES_POOL_MAX>1
-// two clients can both observe a null cache and each run the full pg_type
-// SELECT before either assigns -- harmless duplicate work, last-writer-wins on
-// the shared map.
+// Concurrency: the null check and the `new Map()` assignment in
+// resolveTypeNames are synchronous (no await between them), so the bootstrap
+// SELECT runs at most once per cache lifetime. The actual race with
+// POSTGRES_POOL_MAX>1: a second concurrent caller sees an empty-but-non-null
+// map, skips the bootstrap, and runs the targeted `WHERE oid = ANY($1)`
+// miss-fill for its oids -- harmless duplicate targeted work; both callers
+// merge entries into the same shared Map.
 let typeNameCache: Map<number, string> | null = null;
 
 async function resolveTypeNames(client: pg.PoolClient, oids: number[]): Promise<Record<number, string>> {

src/tools/explain.test.ts

@@ -58,10 +58,10 @@ describe("pg_explain hypothetical_indexes identifier validation", () => {
       hypothetical_indexes: [{ table: 'public."odd.name"', columns: ["x"], using: "btree" }],
     })) as { ok: boolean; error?: string };
     assert.equal(result.ok, false);
-    // The embedded `.` inside the quoted segment splits this into 3 pieces, so
-    // the over-qualified guard fires first; the double-quote guard would catch
-    // it otherwise. Either rejection is correct -- accept both forms.
-    assert.match(result.error ?? "", /double-quote|pre-quoting|over-qualified/i);
+    // The raw-string double-quote check runs before the dot-split, so a
+    // pre-quoted name with an embedded dot gets the actionable "remove the
+    // quotes" message, not the over-qualified one.
+    assert.match(result.error ?? "", /double-quote|pre-quoting/i);
   });
 
   it("rejects an over-qualified (3+ part) table name", async () => {

src/tools/explain.ts

@@ -64,6 +64,15 @@ function quoteQualifiedTable(name: string): string {
  * default re-application in buildHypopgHooks.
  */
 function validateHypoIndex(idx: { table: string; columns: string[] }): string | null {
+  // Check the RAW string for pre-quoting BEFORE splitting on `.`: a pre-quoted
+  // name with an embedded dot (`public."odd.name"`) splits into 3+ pieces, and
+  // the over-qualified message alone gives the caller no hint that removing
+  // the quotes is the fix. JSON.stringify the offending value so a name
+  // containing `"` renders unambiguously instead of producing a broken
+  // nested-quote message.
+  if (idx.table.includes('"')) {
+    return `Hypothetical index table ${JSON.stringify(idx.table)} contains a double-quote; pass plain identifier names without pre-quoting.`;
+  }
   const pieces = idx.table.split(".");
   // Only `schema.table` (1 or 2 pieces) is a legal dotted shape. A 3+ part
   // name like `a.b.c` passes the per-piece checks below but renders as
@@ -73,11 +82,6 @@ function validateHypoIndex(idx: { table: string; columns: string[] }): string |
     return `Hypothetical index table ${JSON.stringify(idx.table)} is over-qualified; use only \`schema.table\` or \`table\`.`;
   }
   for (const piece of pieces) {
-    if (piece.includes('"')) {
-      // JSON.stringify the offending value so a name containing `"` renders
-      // unambiguously instead of producing a broken nested-quote message.
-      return `Hypothetical index table ${JSON.stringify(idx.table)} contains a double-quote; pass plain identifier names without pre-quoting.`;
-    }
     if (Buffer.byteLength(piece, "utf8") > 63) {
       return `Hypothetical index table piece ${JSON.stringify(piece)} exceeds PostgreSQL's 63-byte NAMEDATALEN limit (multi-byte characters count as multiple bytes).`;
     }

src/tools/tools.test.ts

@@ -55,25 +55,26 @@ describe("Tool definitions", () => {
         assert.equal(typeof tool.annotations.openWorldHint, "boolean");
       });
 
-      // Schema/handler-drift guard. Every tool's inputSchema is a ZodObject;
-      // the handler then does `const { ... } = input as { ... }` and reads
-      // the destructured fields without re-validating. A regression that
-      // (a) drops the ZodObject wrapper, (b) leaves the schema empty when
-      // the handler destructures input, or (c) has a schema whose shape
-      // doesn't match the keys the handler reads would compile cleanly
-      // and only surface as `undefined` at runtime -- caught by integration
-      // tests, but months later and at the call site. The cheap probe:
-      // the schema is a ZodObject (already asserted above), the schema
-      // actually parses a sample input built to satisfy each field's
-      // type, and the parsed output exposes the shape's declared keys.
+      // Schema sanity guard. Every tool's inputSchema is a ZodObject; the
+      // handler then does `const { ... } = input as { ... }` and reads the
+      // destructured fields without re-validating. The cheap probe here:
+      // the schema is a ZodObject (already asserted above) and it actually
+      // parses a sample input built to satisfy each field's type -- catching
+      // a dropped ZodObject wrapper or an impossible constraint (a bound or
+      // refine no type-correct input can satisfy). It deliberately does NOT
+      // try to assert that the schema's keys match what the handler
+      // destructures: Zod's parsed output always echoes supplied declared
+      // keys, so a key round-trip assertion can never fail once safeParse
+      // succeeds, and a real handler-key check would need a per-tool fixture
+      // of the keys each handler reads.
       //
       // Note: a tool with `z.object({})` (no declared fields, e.g.
       // pg_list_schemas / pg_list_extensions / pg_replication_status) is
       // a legitimate MCP contract -- the protocol supplies no defaults
       // and these tools take no input. The "must have at least one field"
       // assertion is intentionally NOT made; the test below still runs
       // and passes (parses {}, parsed object is {}) for those tools.
-      it("inputSchema parses a type-correct sample input and round-trips declared keys", () => {
+      it("inputSchema parses a type-correct sample input", () => {
         const shape = tool.inputSchema.shape as Record<string, z.ZodTypeAny>;
         const fieldNames = Object.keys(shape);
 
@@ -134,11 +135,6 @@ describe("Tool definitions", () => {
           true,
           `${tool.name} inputSchema rejected a type-correct sample: ${result.success ? "" : result.error.message}`,
         );
-        if (result.success) {
-          for (const name of fieldNames) {
-            assert.ok(name in result.data, `${tool.name} inputSchema parsed output missing declared field "${name}"`);
-          }
-        }
       });
     });
   }