feat(vrt): add --config flag to qc vrt

[steveworley]

What

Adds qc vrt --config <path> so VRT can read a specific config file instead of always using ~/.quant/vrt-config.json. This lets the config be generated from an external source (e.g. the pulumi-govcms vrtConfig stack output) without clobbering a user's personal config.

pulumi stack output vrtConfig --json --show-secrets > /tmp/vrt.json
qc vrt --config /tmp/vrt.json --format json

Changes

• loadVRTConfigFrom(path) — load/parse a VRT config from an explicit path (null-on-failure, mirrors loadVRTConfig). loadVRTConfig() is now loadVRTConfigFrom(VRT_CONFIG_FILE).
• resolveVRTConfig() — pure, injectable config-source selector (resolves relative --config against cwd). Unit-testable without launching Chromium.
• --config <path> flag + VRTOptions.config; an explicit path that is missing/invalid errors loudly (no silent fallback to the default).
• README documents the flag with a Pulumi example.

Tests

New unit suites (__tests__/utils/vrt-config.test.ts, vrt-resolve-config.test.ts) — 7 tests, no Chromium/network. Unit run green (41/41).

Note: there is a pre-existing, unrelated tsc error at src/utils/api.ts:256 present on main (confirmed via clean-tree check); not touched by this PR.

🤖 Generated with Claude Code

[quantcode-agent]

PR #9 Review — feat(vrt): add --config flag to qc vrt

Scope note: The title is narrow but the diff is broad — it touches CI workflows, package.json/lockfile, tsconfig, command files (app, backup, crawler, env, vrt), utils (api, config), login, and tests. All areas are covered below.

---

Blockers

1. package.json:1 version downgraded — will break npm publish

- "version": "0.4.3",   ← origin/main (base)
+ "version": "0.4.0",   ← HEAD

The base branch is at 0.4.3; this PR regresses it to 0.4.0. Any tag-triggered publish will push a lower version that npm will reject as already-published (or silently shadow a prior release). Looks like a rebase artefact — restore to 0.4.3 (or bump to 0.4.4/0.5.0 as appropriate).

2. .github/workflows/publish.yml — OIDC trusted publishing removed, replaced with long-lived static token

base publish.yml:7-9:

permissions:
  id-token: write  # required for npm OIDC trusted publishing

On HEAD the permissions block is gone entirely, and the publish step now uses NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}.

OIDC provenance tokens are scoped to a single workflow run and cannot be exfiltrated; a static NPM_TOKEN is a long-lived credential that, if leaked from the secrets store, grants permanent publish access. Restore the permissions: id-token: write block and the --provenance publish flag.

3. __tests__/utils/api.test.ts — 395 lines of behavioural tests deleted (524 → 129 lines)

Base: 524 lines covering getApplications, getEnvironments, getMetrics, getLogs, getProjects, getCrawlers, getBackups, the constructor, and 401/403/404 friendly-message paths. HEAD: 129 lines of structural stubs of the form expect(typeof client.getX).toBe('function').

The inline comment says these are "better suited as E2E tests", but the deleted tests mocked the SDK successfully and exercised error-handling branches E2E tests typically don't cover. Deleting them without replacement leaves the error-handling paths untested. Restore the behavioural tests with updated mocks (matching the refactored constructor), or add equivalent coverage before merging.

---

Warnings

W1. package.json:13 — dev script changed from tsx to ts-node --esm

- "dev": "tsx src/index.ts",          ← origin/main
+ "dev": "ts-node --esm src/index.ts", ← HEAD

ts-node is present in HEAD devDependencies (^10.9.1), so npm run dev will still run. However tsx (also in devDeps) handles ESM/CJS interop more reliably for modern TS projects. The regression looks unintentional — confirm it is deliberate.

W2. src/commands/vrt.ts:524-526 — parseBasicAuth returns undefined password for token-only input

function parseBasicAuth(auth: string): { username: string; password: string } {
  const [username, password] = auth.split(':');
  return { username, password };
}

--quant-auth mytoken (no colon) yields password === undefined at runtime despite the declared string type; Playwright httpCredentials then receives { username: 'mytoken', password: undefined }. Add a guard:

if (!auth.includes(':')) throw new Error('--quant-auth / --remote-auth must be in user:pass format');

W3. src/commands/vrt.ts:111-113 — parseFloat/parseInt with no NaN guard

const threshold = parseFloat(options.threshold || config.threshold?.toString() || '0.01');
const maxPages  = parseInt(options.maxPages  || ..., 10);
const maxDepth  = parseInt(options.maxDepth  || ..., 10);

parseFloat('abc') → NaN and downstream comparisons silently misbehave. Add Number.isNaN checks and surface a clear error.

W4. publish.yml / test.yml — action versions downgraded v6 → v4
Base uses actions/checkout@v6 and actions/setup-node@v6 (Node 24); HEAD downgrades both to @v4 (Node 18). v6 of these actions does exist, so this is a deliberate-looking downgrade, not a typo fix. Confirm the Node 24 → 18 change matches the project's supported engine range. Regardless of version, SHA-pinning the actions is recommended for supply-chain hardening.

W5. src/utils/config.ts:239 — parameter path shadows the path module import

// config.ts:3
import { join, resolve } from 'path';
// config.ts:239
export async function loadVRTConfigFrom(path: string): Promise<VRTConfig | null> {

The parameter shadows the module import — a readability hazard that will trip no-shadow lint rules. Rename to filePath. (Note: relative-path resolution is fine — resolveVRTConfig already calls resolve(cwd, configPath) before invoking this function.)

W6. src/commands/env.ts:700 — as any cast for SDK type workaround

// SDK incorrectly types this as void, but it actually returns data
const data = response.data as any;

The comment is honest, but as any disables downstream type checking. Prefer a narrower assertion (as { body?: unknown }) and link the upstream SDK issue so the workaround can be removed later.

---

Test coverage gaps

• Behavioural ApiClient error-handling tests deleted without replacement (see Blocker 3).
• parseBasicAuth has no tests — add cases for no colon, multiple colons (password containing :), empty string.
• No integration test exercising handleVRT with options.config set (the actual --config CLI path).
• No tests asserting graceful rejection of non-numeric threshold/maxPages/maxDepth.

---

Nits (non-blocking)

• vrt.ts help text for --quant-auth / --remote-auth could state the user:pass format so users don't discover it at runtime. Note also that credentials passed as CLI flags appear in ps/shell history/CI logs — the config-file path is the safer place for them.
• tsconfig lib: ES2020 cleanup (dropping DOM) is a sensible tightening — no action needed.

---

Findings withdrawn during verification (for transparency)

• VRTConfig.projects is typed required (config.ts:21), so Object.keys(config.projects) cannot throw on a valid config — no bug.
• AppEnvironment/AppData interfaces are still present in HEAD app.ts — not removed.
• Relative --config paths are resolved correctly by resolveVRTConfig — no bug.

---

Verdict: REQUEST CHANGES

Three blockers must be resolved before merge: the version regression in package.json, the removal of OIDC trusted publishing in favour of a static token, and the deletion of 395 lines of behavioural API tests without replacement. The remaining items are warnings, coverage gaps, and nits.

[steveworley]

Thanks for the thorough review. Root cause of the three blockers: this branch was accidentally cut from a stale local main (9 commits behind origin/main), so the PR diff was surfacing main's own newer work as phantom regressions in this branch. I've rebased onto current origin/main — the branch is now 0 behind / 1 ahead, and the true diff is only the 5 files this feature touches.

Blockers — all resolved by the rebase (none were changes from this PR)

1. Version — back to 0.4.3 (this PR never touched package.json).
2. OIDC publishing — publish.yml retains id-token: write / trusted publishing (this PR never touched CI).
3. api.test.ts — intact at 524 lines (__tests__/utils/api.test.ts, untouched by this PR).

The CI action-version / Node-runtime "downgrade" was the same stale-base artifact and is likewise gone.

Warnings

• path shadows the path module — fixed: the loadVRTConfigFrom parameter is renamed path → filePath.
• as any cast — not present in this PR's diff (grep 'as any' over the 5 changed files is empty); it's pre-existing code elsewhere.
• parseBasicAuth undefined password / missing NaN guards on numeric config — these live in pre-existing main code (vrt.ts / handleVRT), outside this PR's changed lines. Happy to address them in a separate hardening PR so this one stays scoped to the --config flag — let me know your preference.

Verification on the rebased branch

• npx tsc --noEmit — clean (the previously-noted api.ts:256 error was fixed in the rebased commits).
• npm run test:unit — 93/93 pass, including the two new suites.