feat(services,manifest): show the Postgres major as pg 16

[tonychang04]

Why

Migration-cutover report (2026-08-29, item 10): the Postgres major was invisible until connect; a pg_dump from a newer client (17) produced a dump a PG16 server could not restore. The platform now stamps pg_version on postgres service rows and ref.pgVersion on manifest database resources (insta-platform PR, migration 0055).

What

• insta services list: postgres rows show pg 16 (human line). --json was already a verbatim pass-through, so pg_version appears there with the platform change alone.
• insta manifest: database resource lines show pg 16; ManifestResource.ref types pgVersion.
• insta services add postgres <name>: the added line carries the same badge, next to the connect hint.
• One shared pgBadge helper: only a positive integer renders, so true, '16', 16.4, 0, -1 never print as a version.
• Both badges are omitted when the platform sends nothing (older platform, legacy row), so output is unchanged until the platform ships.

Tests

Unit tests for both renderers (present, absent, wrong type). typecheck + the services/manifest/volume suites pass.

Release: bumps package.json to 0.0.57 in this PR so the badge ships as its own release (tag v0.0.57 after merge).

🤖 Generated with Claude Code

https://claude.ai/code/session_017aK2ibJBP6CpD4fRMd3rsT

---

Summary by cubic

Previously, human-readable CLI output hid the Postgres major; it now shows a pg <major> badge so users can choose compatible pg_dump and psql clients before connecting. JSON output remains unchanged.

• Shows the badge in insta services list, insta services add, and insta manifest database rows.
• Reads pg_version from Postgres services and ref.pgVersion from insta-db and legacy neon manifest resources.
• Renders only positive integer versions; missing or malformed values leave existing output unchanged.
• Adds coverage for badge placement and validation, and bumps the CLI version to 0.0.57.

^{Written for commit 4206bf8. Summary will update on new commits.}

[cubic-dev-ai]

No issues found across 4 files

_{Re-trigger cubic}

[jwfing]

Summary
The PR correctly adds a human pg 16 badge to services list and manifest renderers when the platform supplies the new Postgres version fields.

Requirements Context
I used the PR description at #174 as the primary intent source: expose the Postgres major before connect, preserve JSON pass-through, and omit badges for older platform rows with no version. The README only documents these surfaces at a high level as insta manifest being agent-legible and insta services list being part of the service command group (README.md:131, README.md:206-212); I did not find an exact row-format contract in repo docs. The AGENTS.md command/flag doc-mirroring rule does not appear to apply because this PR changes display output only, not commands or flags.

Findings

Critical
(none)

Suggestion

• src/commands/services.ts:141-149, src/commands/manifest.ts:38-41: Consider guarding the new fields with typeof value === 'number' && Number.isInteger(value) before rendering. These values arrive from API JSON as any, so the current truthiness checks would render malformed truthy values like pg true or pg 16 from a string; the PR description also says wrong-type cases are tested, but the checked-in tests only cover present/absent and wrong service/resource placement.

Information

• Software engineering: The change follows the existing pure renderer pattern and adds focused unit tests in test/services.test.ts:166-175 and test/manifest-label.test.ts:50-54. I did not run npm test or npm run typecheck because the review request explicitly required read-only operation and this repo’s tests can create temporary files.
• Functionality: Numeric pg_version / ref.pgVersion values render as claimed, missing values preserve existing output, and --json behavior remains pass-through.
• Security: No security-relevant changes; this only formats existing API response fields for human output, with no new secrets, auth changes, shell/SQL/HTTP construction, or dependencies.
• Performance: No performance-relevant changes; this adds constant-time string formatting to existing list rendering.

Verdict
Approved by the automated verdict rule: no Critical findings. Human GitHub approval is still required by the team’s process.

[jwfing]

Summary

A clean, well-scoped rendering change: two pure renderers gain a pg <major> badge fed by new platform fields, with focused unit tests and no behaviour change until the platform ships — no blocking findings.

Reviewed at head 0bf9061. The head moved during this review (b5f6852 → 9d06f64 → 0bf9061), so this review covers the integer guard added in 0bf9061 in response to the earlier review, and the 0.0.57 version bump in 9d06f64, as well as the original feature commit.

Requirements context

No matching spec/plan found — this repo has no /docs/superpowers/ (no docs/ directory at all; the only prose is README.md, CONTRIBUTING.md, AGENTS.md, and .claude/skills/developing-insta-cli/SKILL.md). Assessed against the PR body, the 2026-08-29 migration-cutover item it cites, and the companion platform PR as the contract of record.

Cross-repo contract verified against insta-platform PR #374 (feat/pg-version-surfacing, migrations/0055_service_pg_version.sql):

• pg_version is Type.Optional(Type.Integer({ nullable: true })) on the shared Service schema (src/openapi/schemas/common.ts), returned by both GET /projects/:id/services (server.ts:1492) and the 201 create (server.ts:1519).
• publicResource attaches ref.pgVersion only for kind === 'neon' | 'insta-db' and only when non-null.
• The CLI's key names, casing (pg_version snake vs ref.pgVersion camel) and types match the platform exactly — no stale or invented field. --json really is verbatim pass-through (src/commands/services.ts:156, printJson(services)), so the body's claim holds.
• Platform #374 is still open and migration 0055 is not on insta-platform main, so the badge is inert until it ships — as the PR body says.
• insta-e2e's services list assertions are substring greps (cleanroom.sh:357-359, grep -q "postgres/db"), so the new badge will not break the cleanroom once the platform ships.

Verification run (clean clone at 0bf9061): npm ci + npm run typecheck clean; npx vitest run → 49 files / 698 tests green. Negative controls: (1) reverting both renderers to their pre-PR form turns the two new "shows the major" tests red; (2) reverting the integer guard to the pre-fix truthiness form turns both new "not an integer" tests red. Both new test groups are load-bearing, not decorative.

Findings

Critical

(none)

Suggestion

1. Functionality — the integer guard admits implausible integers, and it widened what renders vs. the pre-fix check. src/commands/services.ts:150, src/commands/manifest.ts:41

Number.isInteger(0) is true, so the guard now renders a nonsense major where the previous truthiness check omitted it. Probed at this head:

serviceListLine({ type:'postgres', …, pg_version: 0  }) → "postgres/db  [active]  pg 0  svc_pg"
serviceListLine({ type:'postgres', …, pg_version: -1 }) → "postgres/db  [active]  pg -1  svc_pg"
resourceLine({ kind:'insta-db', …, ref:{ pgVersion: 0 } }) → "    - insta-db(db)    pg 0  [active]"

pg 0 / pg -1 is exactly "a version an agent would act on" that the guard's own comment says must not render. Unreachable through the platform contract (the column is stamped from parsePostgresMajor, which rejects anything < 9), which is why this is a Suggestion and not blocking — but Number.isInteger(v) && v > 0 costs nothing and matches the stated rationale. Relatedly, the new bad-value arrays (test/services.test.ts:176-180, test/manifest-label.test.ts:55-59) cover true / '16' / 16.4 / NaN / {} but not 0 — the one value whose behaviour the fix actually changed.

2. Scope — the 0.0.57 version bump doesn't belong in a feature PR. package.json:3, package-lock.json:3,8 (commit 9d06f64)

The documented release flow makes the bump its own PR (.claude/skills/developing-insta-cli/SKILL.md, "Shipping a release" step 1 — "PR changing package.json version"), and main's own history follows it (#173 chore/bump-0.0.56). Bundling it here makes the 0.0.57 release contingent on this feature landing, and erases the bump PR as the release boundary — any feature merged after this one silently rides 0.0.57 with nothing marking it. No functional impact (cliVersion() only feeds --version / the User-Agent, and the upgrade comparator is unaffected while npm latest is behind), so this is process drift rather than a defect.

3. Functionality — insta db connect is where the incident actually bit, and it still doesn't name the major. src/commands/db.ts:320-332 (banner at :330), src/commands/db.ts:245-263

The motivating failure was a client/server major mismatch at dump/connect time, and resolveDbUrl already fetches the very services row that carries pg_version (GET /projects/:id/services?branch=…) before spawning psql. Adding it to the existing stderr banner (psql → postgres/<name> …) is nearly free and closes the loop for the reader who never runs services list first. Out of this PR's stated scope, so non-blocking — but it's the cheapest remaining half of the feature.

4. Functionality/consistency — insta services add postgres <name> omits the badge. src/commands/services.ts:120-135

The human line prints access / region / image / volume / domain and then the insta db url|connect hint, but not the major — even though the 201 create response carries pg_version on the same shared Service schema (server.ts:1519, ServicesRepo.create does returning *). The badge is absent exactly where the connect hint is printed.

5. Software engineering / convention — mirror the badge in skills/insta/cli-reference.md.

AGENTS.md non-negotiable #4's letter is about command/flag changes, so I agree this PR isn't strictly covered by it. But that reference is the agent-facing surface doc, agents are the stated audience for this badge, and it already documents comparable output facts (cli-reference.md:17: "The image is persisted on the service — shown in services list"). Surfacing a field that nothing tells agents to look for gets you half the value.

Information

6. Label divergence: insta db stats already renders the Postgres version as PG <serverVersion> (src/commands/db.ts:154-157, e.g. PG 16.4); this PR spells the same concept pg 16. Both are agent-facing output from the same CLI — one spelling (or a cross-reference in the doc) would help a grepping agent find both.

7. src/commands/manifest.ts:13-14 says pgVersion is "the Postgres MAJOR a database row runs". The platform's contract is weaker: migration 0055 stamps it at create from the configured plane major and carries it on clones, never updating it — "existing rows keep the major they were born with". After an in-place plane image bump the badge would state a stale major with full confidence. Wording like "recorded at create" would match the contract. (No platform major-upgrade endpoint exists today, so this is forward-looking only.)

8. No test renders where and the badge together (e.g. ref: { neonProjectId: 'p1', pgVersion: 16 }). Low value in practice: the platform attaches pgVersion to neon rows too, but Neon is retained-not-live, and live insta-db rows produce an empty where — so the combination is effectively unreachable today.

9. With an empty where, the line renders with four spaces ( - insta-db(db) pg 16 [active]). Pre-existing — the badge just lands in that gap — and test/manifest-label.test.ts:53 pins it, so noting only.

Security

No security-relevant changes. Two platform-supplied scalars are interpolated into human output only: no new user input reaching SQL/shell/HTTP, no auth or authorization check touched, no secrets/tokens/PII added to output (--json remains verbatim printJson(services), so the CLI adds no field to any response), and no new or changed dependencies — the only package.json edit is the version string.

Performance

No performance-relevant changes in this PR. Both additions are O(1) string formatting inside renderers that already run once per row; no new requests, no new allocations of note, nothing added to a hot path or the event loop. (Cross-repo note, not this PR's to fix: the platform side pays one extra ServicesRepo.listByProject per GET /projects/:id to build the pgVersion map — a single query, not an N+1.)

Verdict

approved — zero Critical findings. (Informational: the GitHub green checkmark is still a separate human action via the approve flow.) The Suggestions worth picking up before merge are #1 (&& v > 0, plus a 0 case in the bad-value arrays) and #2 (split the version bump out).

[jwfing]

Summary
The PR is a small display-only change that fits the existing pure-renderer pattern and has focused unit coverage.

Requirements context
I used the PR title/description as the primary intent: show the Postgres major as pg 16 in insta services list and insta manifest, omit it when absent or malformed, and keep --json as verbatim API output. Repo context confirms insta manifest is intended as an agent-legible project view (README.md:131-132) and that pure command renderers are unit-tested in place (src/commands/services.ts:139-141, src/commands/manifest.ts:37-38). I did not find a separate local output schema for these lines.

Findings

Critical
(none)

Suggestion

• src/commands/manifest.ts:39-42: Consider gating the manifest pgVersion badge to database resources, mirroring the s.type === 'postgres' guard in src/commands/services.ts:142-150. The platform contract says ref.pgVersion is stamped on manifest database resources, so this is not blocking, but the current renderer would print pg N on any resource row that happened to carry an integer ref.pgVersion.

Information

• Software engineering: Tests cover present, absent, and wrong-type values for both renderers (test/services.test.ts:166-180, test/manifest-label.test.ts:50-58), and the implementation follows the existing pure-helper style (src/commands/services.ts:139-151, src/commands/manifest.ts:37-42).
• Functionality: Human output gains the intended badge while services list --json and manifest --json still return raw API bodies (src/commands/services.ts:154-161, src/commands/manifest.ts:46-55).
• Security: No security-relevant changes found; this only formats existing API fields and does not add SQL, shell execution, auth changes, secret output, or dependencies (src/commands/services.ts:141-151, src/commands/manifest.ts:38-42, package-lock.json:11-24).
• Performance: No performance concern found; the added work is constant-time formatting inside existing output loops (src/commands/services.ts:141-161, src/commands/manifest.ts:52-55).
• Verification: npm run typecheck could not run locally because node_modules/.bin/tsc and node_modules/.bin/vitest are missing; I did not install dependencies due to the read-only review constraint.

Verdict
Approved per the requested verdict rule: no Critical findings.

[cubic-dev-ai]

All reported issues were addressed across 4 files (changes from recent commits).

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[jwfing]

Summary
The implementation matches the claimed human-output change, with one non-blocking test coverage gap around the services add success line and no security or performance concerns.

Requirements Context
I derived the intent from the PR title and description: show pg <major> in human output for insta services list, insta services add postgres <name>, and insta manifest database rows, using pg_version and ref.pgVersion, while omitting absent or malformed values and leaving JSON pass-through unchanged. Repo context says insta manifest is agent-legible (README.md:129-134) and command/flag changes must be mirrored in the external CLI reference (CONTRIBUTING.md:36-44, .claude/skills/developing-insta-cli/SKILL.md:34-39); I did not find a checked-out skills/insta/cli-reference.md, and this PR does not add or rename commands or flags.

Findings

Critical
(none)

Suggestion

• src/commands/services.ts:123-129, test/services.test.ts:166-180 - insta services add postgres badge output is assembled inline and is not directly tested. serviceListLine covers list rows and resourceLine covers manifest rows, but a future edit could drop pgBadge(svc.pg_version) from the add success line without the current changed tests failing. Consider extracting that line to a pure renderer or adding a focused fake around servicesAdd.

Information

• src/commands/services.ts:141-158, src/commands/manifest.ts:39-43 - Functionality, security, and performance check: the badge renderer only uses already-returned API fields, filters malformed values with a positive-integer guard, keeps JSON paths unchanged, and adds no dependencies, network calls, SQL/shell execution, unbounded loops, or blocking work.
• test/manifest-label.test.ts:50-62, test/services.test.ts:166-180 - Software engineering check: the changed list and manifest renderers have focused unit coverage for present, absent, and malformed version values. I could not run local verification because tsc and vitest are not installed in this checkout, and I did not install dependencies under the read-only review constraint.

Verdict
Approved by the requested severity rule: there are no Critical findings.

[jwfing]

Summary

A display-only change that adds a pg <major> badge to three human output surfaces behind one shared positive-integer guard; the guard is now correct, and the two pure renderers are genuinely covered — the services add surface is not.

This is a re-review at head 5181808, additive to my earlier round at 0bf9061. Three commits have landed since (5b3cf98 services add badge + v > 0, b60ac29 manifest kind gate, 5181808 test naming), so the substance below is about that new code.

Requirements context

No matching spec/plan found — assessing against the PR description alone. insta-cli has no docs/ directory at all (only README/CONTRIBUTING/AGENTS.md/CLAUDE.md + .claude/skills/developing-insta-cli/SKILL.md), so there is no /docs/superpowers/ spec to hold this to. Intent was taken from the PR body (migration-cutover report 2026-08-29 item 10) and from the companion platform PR.

Verification performed in a clone at 5181808:

• npm ci + npx tsc --noEmit clean; npx vitest run 699 passed / 49 files.
• Field shape confirmed against the open companion insta-platform#374 (head ac998d75), not guessed: pg_version is Type.Optional(Type.Integer({ nullable: true })) on the shared Service schema (src/openapi/schemas/common.ts:255), and publicResource attaches ref.pgVersion under exactly (r.kind === 'neon' || r.kind === 'insta-db') && extra.pgVersion != null — root and branch rows alike. So this PR's manifest gate (manifest.ts:42) and its ref.pgVersion comment (manifest.ts:14-15) both match the platform precisely, including the "root and branch rows alike" claim.
• Reachability of all three surfaces confirmed: platform GET /projects/:projectId/services and POST .../services both type their response with Type.Ref(S.Service) (server.ts:1492, :1518), and the CLI passes rows through whole (services.ts:168 info(serviceListLine(s))), so nothing is stripped and no surface is dead code. The --json verbatim-passthrough claim also checks out (services.ts:166, manifest.ts:51).
• pg_version / pgVersion has zero occurrences on insta-platform@main and its migrations/ stop at 0054 — the platform half has not merged. That matters only for shipping order (Suggestion 2), not for correctness here.

Findings

Critical

(none.)

The 0 / -1 widening I raised last round is fixed and now tested. Negative control: reverting pgBadge to the pre-change form (typeof v === 'number' && Number.isInteger(v), dropping && v > 0) turns both new bad-value tests red — serviceListLine > omits the badge when pg_version is not a positive integer and resourceLine > omits the badge when ref.pgVersion is not a positive integer. The arrays now include 0 and -1, the two values whose behaviour that fix actually changed. A second NC on b60ac29 — deleting the r.kind === 'insta-db' || r.kind === 'neon' gate at manifest.ts:42 — turns resourceLine > never badges a non-database row, whatever its ref carries red. Both new guards bind.

Suggestion

1. Software engineering / test coverage — the services add badge is entirely untested (src/commands/services.ts:127-129).

Negative control: deleting ${pg} from the info(...) template at services.ts:129 leaves the full suite green — 699 passed, 49 files, zero failures. The newest feature surface in the PR, and an explicit claim in the body ("the added line carries the same badge, next to the connect hint"), is guarded by nothing. It is reachable (POST response is Type.Ref(S.Service), so svc.pg_version does arrive), so this is a coverage gap rather than dead code — but the badge's placement is the whole point of the commit, and placement is exactly what an untested template string silently loses.

Worth noting the two pure renderers were done right — pgBadge reuse means the guard is covered transitively; it's the composition (${vol}${pg}${svc.domain ...} ordering, and the svc.type === 'postgres' gate versus the surrounding type === 'postgres' hint gate) that isn't. The repo already owns the pattern: test/volume.test.ts, test/limits.test.ts, test/db-url.test.ts and test/compute-domain-flow.test.ts all mock ApiClient and assert on info() output, and test/services.test.ts:145 already imports servicesAdd. One case asserting the added line contains pg 16 for a postgres response, and one asserting it does not for storage, would close it.

2. Functionality / shipping order — the badge is inert until insta-platform#374 merges, and the docs mirror is already ahead of both.

Not a defect: pgBadge(undefined) === '', so pre-#374 output is byte-identical, which is exactly the fail-quiet behaviour the body claims and which I verified. But AGENTS.md non-negotiable 4 requires command surface changes to be mirrored in skills/insta/cli-reference.md, and that mirror (insta-skills#72) documents pg_version / ref.pgVersion as present. Since main of insta-skills is the production skill source, tagging v0.0.57 before platform #374 lands would make that doc true of no deployment. Suggested order: platform #374 → this PR → tag v0.0.57 → insta-skills#72. Flagging the coupling because three repos have to move in sequence, not because anything here is wrong.

Information

1. pg 16 diverges in case and precision from the CLI's only existing Postgres version label. src/commands/db.ts:156 renders PG ${body.serverVersion} — uppercase and the full server version (e.g. PG 16.4). This PR renders lowercase pg 16, major only. Both are defensible in isolation (the badge is deliberately the major, because the major is what picks pg_dump), and the badge's lowercase form matches its neighbours (vol 10Gi, running …, tcp/6379), so I would not change it — but an agent grepping CLI output for a Postgres version now has two shapes to match. If the difference is intentional, db.ts:156 is the place a one-line comment would earn its keep.

2. Version bump bundled into the feature PR. .claude/skills/developing-insta-cli/SKILL.md:49 frames the release's first half as "Bump: PR changing package.json version". Bundling it means merging the feature immediately arms the v0.0.57 tag rather than leaving that a separate decision — mildly at odds with Suggestion 2's ordering. The body discloses and justifies it, which is the important part. Mechanically it is safe: registry.npmjs.org/insta dist-tags.latest is 0.0.56 and 0.0.57 is unpublished, so there is no collision, and the package-lock.json change is the two version strings only — no dependency graph movement.

3. serviceListLine's extra chain is now four ternary arms deep (services.ts:151-155). Still readable, and it matches the file's existing style, so this is a note rather than a request — but the next service type to earn a suffix is probably the one that should convert it to a switch or a per-type lookup.

4. No security-relevant changes in this PR. No new user input reaches SQL, shell, or HTTP — pgBadge only ever formats a platform-supplied number into an output string, and it is total on unknown. Nothing new is logged or returned: pg_version is not a secret, and the badge adds no credential-bearing field (publicResource's insta-db ref carries instanceId/routeKey, unchanged here). No auth or authorization path is touched, and no dependencies are added.

5. No performance-relevant changes on the CLI side. Two O(1) string helpers on an already-iterated result set — no new request, no new loop, no added allocation per row. (The extra ServicesRepo.listByProject read that #374 adds to getDetail is that PR's to assess, and it is a single batched query building a Map, not an N+1.)

Verdict

approved — zero Critical findings. Typecheck and the full 699-test suite are green at 5181808, the field contract matches the companion platform PR exactly, all three surfaces are reachable, and the previous round's widening bug is fixed with tests that provably fail without the fix. The one gap worth closing before merge is Suggestion 1 — the services add badge is the only new behaviour that survives deletion with a green suite.

(Informational: the green checkmark is a separate human action.)

[jwfing]

Summary
The PR cleanly adds a guarded Postgres major badge to human service and manifest output without changing command flags or JSON pass-through.

Requirements context
I used the PR title/description as the primary intent: render pg 16 for platform-provided Postgres majors in insta services list, insta services add postgres, and insta manifest; omit the badge for missing or malformed values; keep --json verbatim. Local docs establish insta as a thin API client and insta manifest as agent-legible (README.md:10-11, README.md:131-134), and require command/flag changes to update the external CLI reference (CONTRIBUTING.md:41-44); this PR does not change commands or flags. I also checked the linked external CLI reference: https://raw.githubusercontent.com/InsForge/insta-skills/main/insta/cli-reference.md.

Findings

Critical
(none)

Suggestion
(none)

Information

• Software engineering/functionality: src/commands/services.ts:136-164 and src/commands/manifest.ts:39-43 implement the described behavior with one shared pgBadge guard, gated to Postgres/database rows, while preserving the existing no-badge output shape. The added tests cover present, absent, malformed, and non-Postgres/non-database cases in test/services.test.ts:166-192 and test/manifest-label.test.ts:50-61.
• Security/performance: src/commands/services.ts:150-164 and src/commands/manifest.ts:39-43 only format already-fetched API fields. There are no new SQL, shell, HTTP, auth, secret, dependency, unbounded-loop, or blocking-I/O paths.
• Verification note: package.json:36-39 defines the expected typecheck/test commands, but local execution failed because tsc and vitest were not installed in this checkout. I did not install dependencies under the read-only review constraint.

Verdict
Approved per the requested severity rule: no Critical findings. Human approval is still separate from this bot verdict.

[jwfing]

Summary

Round 3, at head 4206bf8. The single new commit (test(services): pure serviceAddedLine renderer…) closes the one Suggestion I left at 5181808 — the services add badge placement is now a pure, asserted renderer. I re-ran every gate and every negative control from rounds 1–2 against this head; all guards bind, the extraction is provably behaviour-preserving, and I found nothing new. No Critical findings.

Requirements context

No matching spec/plan found — insta-cli has no docs/ directory at all at this head (ls -d docs* → nothing; the only in-repo guidance is .claude/skills/developing-insta-cli). Assessed against the PR title/body plus the real contract this PR consumes: the companion platform PR InsForge/insta-platform#374, re-read at its current head ba09e3c rather than trusting my round-2 notes. The field shapes the CLI depends on still match exactly:

CLI read	Platform #374
s.pg_version / svc.pg_version (snake)	pg_version: Type.Optional(Type.Integer({ nullable: true })) on the shared Service TypeBox schema
r.ref?.pgVersion (camel), gated kind === 'insta-db' || 'neon'	publicResource attaches ref.pgVersion under exactly (kind === 'neon' || kind === 'insta-db') && extra.pgVersion != null
services add badge (services.ts:124)	ServicesRepo.create adds pg_version to cols and returns insert … returning *; the POST response is typed Type.Ref(S.Service), so fastify no longer strips it

Naming, casing and gating all line up on both planes. insta services list --json is printJson(services) verbatim (services.ts:172), so the body's "JSON unchanged" claim holds by construction.

Findings

Critical

(none)

Suggestion

(none) — my round-2 Suggestion is closed; see the first Information item for the proof.

Information

Software engineering — round 2's coverage gap is genuinely closed (proved, not read). At 5181808 I reported that deleting the ${pg} segment from the inline services add template left the entire suite green. Re-running that exact sabotage at this head now reds:

- return `…${img}${vol}${pg}${svc.domain ? ` — ${svc.domain}` : ''}`
+ return `…${img}${vol}${svc.domain ? ` — ${svc.domain}` : ''}`
  × serviceAddedLine > carries the Postgres major next to the connect hint for a postgres service
  Tests  1 failed | 700 passed (701)

Full NC sweep at 4206bf8 — all five bind, none is decorative:

#	Sabotage	Result
A	drop ${pg} from serviceAddedLine (services.ts:144)	✅ reds serviceAddedLine > carries the Postgres major…
B	drop the svc.type === 'postgres' gate (services.ts:143)	✅ reds serviceAddedLine > never badges a non-postgres service…
C	drop && v > 0 from pgBadge (services.ts:151) — round 1's finding	✅ reds 2 tests (services + manifest)
D	drop the kind === 'insta-db' || 'neon' gate (manifest.ts:42)	✅ reds resourceLine > never badges a non-database row…
E	drop the postgres arm of serviceListLine (services.ts:163)	✅ reds serviceListLine > shows the Postgres major…

Functionality — the extraction is behaviour-preserving, verified exhaustively. Moving a live template into a new function is the classic place to silently lose a segment, so I differential-tested serviceAddedLine (services.ts:138-145) against main@559d562's inline template verbatim across the full cross-product of type × branch × public × image × port × volume_gib × region × domain × 10 pg_version values: 23,040 cases, 0 drift — every output identical except the intended pg <major> insertion, in the intended position, on exactly the postgres rows where the guard admits the value.

Gates — run, not assumed. npm ci clean; npx tsc -p tsconfig.json --noEmit exits 0 at head and at base 559d562 (so no pre-existing noise to misattribute); npx vitest run → 701 passed / 49 files, up from the 699 I measured at 5181808. (The prior review at this same head noted it could not run these locally — they pass.)

Functionality — the badge is inert until the platform ships, as the body claims. insta-platform#374 is still open, and pg_version/pgVersion have 0 occurrences on insta-platform@main. Until it merges, every surface here receives undefined and renders nothing — which NCs C/D/E confirm is a guarded no-op, not an accident. Shipping order remains: platform#374 → this PR → tag v0.0.57 → insta-skills#72 (which asserts these fields and is blocked on the first two).

Release — no version collision. npm dist-tags.latest for insta is still 0.0.56; 0.0.57 is unpublished, so the in-PR bump arms the tag cleanly on merge.

Two carried-over notes, restated for the record, not re-litigated:

• insta db stats renders the live server version as PG <serverVersion> (src/commands/db.ts:156) — uppercase and full — while this badge is lowercase pg 16. Different sources (live metrics vs. the row's recorded major), so the divergence is defensible; flagging only so a future consistency pass knows both exist.
• The neon arm of manifest.ts:42 is unreachable in practice — platform #374 stamps pgVersion only when kindFor('postgres') === 'insta-db', so retained Neon rows always carry null. Harmless defensive breadth.
• Residual, and explicitly not a request: nothing drives the info(serviceAddedLine(…)) wiring at services.ts:124 end-to-end (servicesAdd tests stop at the pre-network throws). That is the repo's established pattern for every renderer — serviceListLine, resourceLine, volumeLines, billingLines are all tested pure — and the segment-loss risk I actually raised is now covered.

Security — no security-relevant changes. No new SQL, shell, HTTP, auth, dependency or secret paths; nothing new is logged or returned. Worth noting as a positive: because pgBadge requires typeof v === 'number' (services.ts:151), a hostile or corrupted API value — including a string carrying ANSI escapes — can never reach the terminal through this path. A truthiness check would have passed it through.

Performance — nothing. Three pure O(1) string helpers on already-fetched data, one call per rendered row. No new queries, allocations, loops, or I/O.

Verdict

approved — zero Critical findings, zero open Suggestions. (Informational: the green checkmark is still a separate human action.) The delta since my last round is a targeted test-quality fix that does exactly what it claims, and I verified it by sabotage rather than by reading it.

[jwfing]

Summary

Round 4, at head 4206bf8 — unchanged since my round-3 review (#pullrequestreview-5104696041), with no new commits and no rebuttal. This round adds independent re-verification rather than new findings: all gates and all five negative controls were re-run from a clean clone. No Critical findings.

Head/base pinning. git ls-remote (not the PR API, which can lag a commit) reports feat/pg-version-badge = 4206bf8 and main = 559d562 — both identical to r3, and the 7-commit list is unchanged. The diff is byte-identical to what I reviewed at r3.

Requirements context

No matching spec/plan found — assessing against the PR description alone. This repo has no docs/ directory at all, so there is no /docs/superpowers/ (or docs/specs/) plan to check against; the only in-repo convention source is .claude/skills/developing-insta-cli/SKILL.md plus CLAUDE.md/AGENTS.md. Intent is therefore taken from the PR title/body and the cited migration-cutover item 10 (the Postgres major was invisible until connect, so a pg_dump from a 17 client produced a dump a PG16 server could not restore).

Verification performed this round

From a fresh clone at 4206bf8, after npm ci:

Check	Result
npm run typecheck (tsc --noEmit)	exit 0
npx vitest run	701 passed / 701, 49 files
5 negative controls	all 5 bind (below)

Negative controls — each mutation applied with an asserted single-match edit, suite re-run, then reverted (git status clean after each):

1. Drop ${pg} from the serviceAddedLine template → reds serviceAddedLine > carries the Postgres major next to the connect hint… (1 failed / 700).
2. Drop the svc.type === 'postgres' gate in serviceAddedLine → reds never badges a non-postgres service….
3. Drop && v > 0 from pgBadge → reds two tests, one per surface (serviceListLine and resourceLine positive-integer cases).
4. Drop the r.kind === 'insta-db' || r.kind === 'neon' gate in resourceLine → reds never badges a non-database row, whatever its ref carries.
5. Drop the postgres arm from serviceListLine → reds shows the Postgres major on a postgres row….

This closes the coverage gap I raised at 5181808: the services add badge placement (not just the shared guard) is now asserted by NC 1, which previously red nothing.

Findings

Critical

(none)

Suggestion

(none new.) The one Suggestion I left in round 2 — the services add surface being guarded by nothing — was closed by 4206bf8 and is confirmed closed by NC 1 above.

Information

• Software engineering — src/commands/manifest.ts:2 importing pgBadge from ./services.js is idiomatic here, not a layering smell: cross-command imports are established precedent (compute.ts, db.ts, db-query.ts, storage.ts, billing.ts). I re-checked for a cycle this round: services.ts imports only ../api.js and ../util.js, so the new edge is acyclic.
• Functionality — the neon arm at src/commands/manifest.ts:42 is defensive but unreachable in practice: the platform stamps pgVersion only when the postgres kind resolves to insta-db. Harmless as legacy tolerance.
• Functionality — insta db stats renders PG <serverVersion> (src/commands/db.ts:156) — uppercase, full version, from live metrics — versus pg 16 (recorded major) here. A defensible divergence given the different sources; noting it only so the inconsistency is a choice rather than an accident.
• Release — the 0.0.57 bump is collision-free: npm dist-tags.latest for insta is still 0.0.56 and 0.0.57 is unpublished. package.json and package-lock.json are bumped together (both root version and packages[""].version), so npm ci stays consistent.
• Dependency ordering — the companion platform PR (insta-platform#374) is still open at ba09e3c, so pg_version / ref.pgVersion are not yet on the wire. That is safe by construction: every badge is omit-by-default, so human output is byte-identical until the platform ships. Shipping order remains platform#374 → this PR → tag v0.0.57.

Dimension coverage

• Software engineering — both renderers are pure and unit-tested, matching the repo's established pattern (serviceListLine, resourceLine, volumeLines, billingLines); command wiring is never tested end-to-end in this repo, so the untested info(...) call site is convention, not a gap. Import style, naming, and comment density match the surrounding code.
• Functionality — verified both real call sites pass raw API objects through with no field-picking mapper that could drop the new fields: servicesList does for (const s of services) info(serviceListLine(s)), and manifest does for (const r of rs) info(resourceLine(r)) over detail.resources. --json on both paths is a verbatim printJson pass-through, so JSON output is unchanged by this PR and gains pg_version from the platform change alone — matching the PR's claim. The guard accepts exactly the positive integers: 0, -1, 16.4, NaN, Infinity, true, '16', {} are all rejected.
• Security — no security-relevant changes. No new user input reaches SQL/shell/HTTP; no new dependency; nothing newly logged or returned. The badge renders a small integer the platform already owns. (ref.url printing in resourceLine predates this PR and is untouched.)
• Performance — no performance-relevant changes. No new requests, loops, or allocations; the badge is one constant-time string concat per already-rendered row.

Verdict

approved — zero Critical findings. Gates and negative controls all green at 4206bf8 from a clean clone. (Informational: the explicit GitHub approval remains a separate human action.)

[jwfing]

LGTM - approved.