[AI-1283] Report the new CLI version to the server after kcap update

[realtonyyoung]

Part of AI-1283 — companion to kcap-server#1389. Together they fix a reported bug: after kcap update, the "your kcap CLI is out of date" web banner/notification linger.

Gap this closes

The server only learns your installed CLI version from an authenticated CLI→server request carrying X-Kcap-Cli-Version. kcap update itself makes no such request (it only talks to the npm registry), so the server didn't re-observe the new version until your next incidental hook/status/daemon call — leaving the out-of-date surfaces stale in the meantime.

Fix

• New hidden report-version command: makes ONE quiet, fail-open authenticated GET to a side-effect-free probe (/api/me/notification-prefs, the same read whoami uses) via the header-carrying client, so the server's observer middleware records the new version from the X-Kcap-Cli-Version header. Never prints on the happy path, always returns 0, bounded ~5s. Skips silently when not authenticated; proceeds on both Ok and NoAuthRequired tenants; listed in offlineCommands so a no-server host still returns 0.
• The npm wrapper (bin/kcap.js runUpdate) invokes the new binary's report-version after a successful install/refresh and before exit, best-effort (stdio: "ignore", timeout, try/catch) — it can never change the update's exit code.

Deliberately a side-effect-free GET, not the /api/users/me/cli-setup POST: that endpoint fires a one-time onboarding event, which would falsely mark a login-but-never-setup user as "Registered" (caught in review).

Notes

• No Linear IDs in any .cs (repo lint).
• Tests: ReportVersionCommandTests 8/8 (header-on-GET, not-authed→no-request, no-auth-tenant, no-server, error/timeout all return 0), observation-header + update-notice suites unaffected.

🤖 Generated with Claude Code

[linear-code]

AI-1283

[qodo-code-review]

PR Summary by Qodo

Report new CLI version to server immediately after kcap update

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Add hidden kcap report-version to send one authenticated, side-effect-free probe request.
• Spawn report-version after a successful npm update to clear stale “out of date” banners.
• Ensure the report is silent, fail-open, offline-safe, and covered by focused unit tests.

Diagram

graph TD
  A(("User")) --> B["npm wrapper: runUpdate"] --> C["new CLI cmd: report-version"] --> D["Auth client adds X-Kcap-Cli-Version"] --> E{{"Server: GET /api/me/notification-prefs"}} --> F["Observer records version / clears notice"]

  subgraph Legend
    direction LR
    _actor(("Actor")) ~~~ _proc["CLI step"] ~~~ _ext{{"Server/API"}}
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Add dedicated server ping endpoint for version observation

• ➕ Clear contract: endpoint exists specifically to record version, no ambiguity
• ➕ Avoids coupling to notification-prefs/whoami semantics
• ➕ Could support future telemetry/health checks cleanly
• ➖ Requires coordinated server changes and deployment
• ➖ More surface area (authz, routing, docs) than reusing an existing read endpoint

2. Call existing `/api/users/me/cli-setup` POST after update

• ➕ Already exists; likely to traverse the same auth/header middleware
• ➖ Has behavioral side effects (one-time onboarding event) and can corrupt user state
• ➖ Harder to reason about idempotency given it runs on every update

3. Do nothing; rely on next incidental authenticated request

• ➕ No new code paths; simplest operationally
• ➖ Leaves stale “CLI out of date” UI until the next server-touching command
• ➖ Confusing UX and increases support noise

Recommendation: Keep the PR’s approach: a single authenticated, side-effect-free GET via the standard header-attaching client, executed best-effort after update. It minimizes server coupling, avoids onboarding side effects, and is safely fail-open (timeouts/try-catch, offline-safe). If this pattern expands, consider a dedicated server ping endpoint later for clarity.

Files changed (7) +331 / -3

Enhancement (1) +64 / -0

ReportVersionCommand.csIntroduce hidden 'kcap report-version' command (fail-open probe) +64/-0

Introduce hidden 'kcap report-version' command (fail-open probe)

• Adds a tooling-internal command that creates an authenticated client (ensuring the CLI version header is attached) and performs a single GET to the whoami probe path ('/api/me/notification-prefs'). It silently no-ops unless auth is Ok/NoAuthRequired, uses a short timeout, swallows all errors, and always returns 0.

src/Capacitor.Cli/Commands/ReportVersionCommand.cs

Bug fix (3) +26 / -2

kcap.jsSpawn 'report-version' after successful update refresh +14/-0

Spawn 'report-version' after successful update refresh

• After 'npm install -g' and refreshes succeed, the wrapper best-effort executes the freshly installed binary with 'report-version --no-update-check'. The call is silent, timeout-bounded, and guarded so it can’t affect 'kcap update' exit status.

npm/kcap/bin/kcap.js

CrashReporter.csMark 'report-version' as fail-open on crash +1/-1

Mark 'report-version' as fail-open on crash

• Adds 'report-version' to the set of commands that must exit 0 even if a crash occurs, matching its fire-and-forget semantics.

src/Capacitor.Cli/CrashReporter.cs

Program.csWire 'report-version' into offline allowlist and command dispatch +11/-1

Wire 'report-version' into offline allowlist and command dispatch

• Adds 'report-version' to the offlineCommands list so hosts without a configured server don’t fail early with “No server configured”. Adds a hidden command case to dispatch into 'ReportVersionCommand.HandleAsync'.

src/Capacitor.Cli/Program.cs

Tests (2) +239 / -0

kcap.test.jsAdd source-shape unit test for post-update 'report-version' spawn +37/-0

Add source-shape unit test for post-update 'report-version' spawn

• Adds an assertion-based test that inspects 'kcap.js' source to verify 'execFileSync(report-version ...)' exists, is ordered after refresh and before exit, and is protected by its own try/catch. This avoids needing to mock 'child_process' or actually run npm installs in unit tests.

npm/kcap/bin/kcap.test.js

ReportVersionCommandTests.csAdd comprehensive unit tests for 'ReportVersionCommand' behavior +202/-0

Add comprehensive unit tests for 'ReportVersionCommand' behavior

• Introduces a WireMock-based test suite validating the command sends exactly one GET with the version header when authenticated (including NoAuthRequired tenants). Verifies not-auth/offline/unreachable/slow/error cases all return 0 and do not produce extra requests.

test/Capacitor.Cli.Tests.Unit/ReportVersionCommandTests.cs

Documentation (1) +2 / -1

UpdateNotice.csDocument update-notice suppression for 'report-version' +2/-1

Document update-notice suppression for 'report-version'

• Updates the suppression predicate documentation to include 'report-version' among commands where update notices should not be printed to stderr.

src/Capacitor.Cli/UpdateNotice.cs

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Wrong host probe URL ✓ Resolved 🐞 Bug ⛨ Security

Description

ReportVersionCommand builds the probe URL without considering KCAP_URL, but
CreateClientWithAuthStatusAsync *does* default to KCAP_URL. If HandleAsync is invoked with
baseUrl == null while KCAP_URL is set, the client may authenticate for KCAP_URL but send the
GET (with Authorization) to http://localhost:5108 instead.

Code

src/Capacitor.Cli/Commands/ReportVersionCommand.cs[R51-54]

+                var url = AppConfig.NormalizeUrl(baseUrl ?? AppConfig.ResolvedServerUrl ?? "http://localhost:5108")
+                        + WhoamiCommand.ProbePath;
+
+                using var _ = await client.GetOnceAsync(url, RequestTimeout);

Evidence

ReportVersionCommand’s URL fallback omits KCAP_URL, while CreateClientWithAuthStatusAsync resolves
baseUrl by including KCAP_URL; therefore, when HandleAsync is called with null baseUrl and
KCAP_URL set, the resolved authenticated target can differ from the actual request URL.

src/Capacitor.Cli/Commands/ReportVersionCommand.cs[44-55]
src/Capacitor.Cli.Core/HttpClientExtensions.cs[73-77]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`ReportVersionCommand.HandleAsync` computes the probe URL using `baseUrl ?? AppConfig.ResolvedServerUrl ?? "http://localhost:5108"`, but the authenticated client builder (`CreateClientWithAuthStatusAsync`) resolves `baseUrl` using `... ?? KCAP_URL ?? "http://localhost:5108"`. This mismatch can send an authenticated request (including the bearer token) to the wrong host.

### Issue Context
You want *one* resolved server URL used consistently for:
1) provider discovery/token selection/auth header, and
2) the probe request URL.

### Fix
- Compute a single `effectiveBaseUrl` in `ReportVersionCommand.HandleAsync` using the same fallback chain as `HttpClientExtensions` (include `Environment.GetEnvironmentVariable("KCAP_URL")`).
- Pass `effectiveBaseUrl` into `CreateClientWithAuthStatusAsync`.
- Build the probe URL from `effectiveBaseUrl` (normalized) + `WhoamiCommand.ProbePath`.

### Fix Focus Areas
- src/Capacitor.Cli/Commands/ReportVersionCommand.cs[44-55]
- src/Capacitor.Cli.Core/HttpClientExtensions.cs[73-77]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Verbose runUpdate comment block ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

New multi-paragraph comments add extensive design rationale inline, duplicating intent that could be
conveyed with shorter wording and clearer code structure. This reduces maintainability by making key
logic harder to scan and increasing future update burden.

Code

npm/kcap/bin/kcap.test.js[R111-114]

+// runUpdate's post-refresh version report to the server: `execFileSync` is destructured from
+// `child_process` at module load (`const { execFileSync, spawnSync } = require("child_process")`),
+// so there is no seam to intercept it short of restructuring the module or adding a mocking
+// dependency this package doesn't otherwise carry (no proxyquire/sinon/jest here — see

Evidence

PR Compliance ID 5 requires keeping comments minimal and relying on code clarity rather than lengthy
commentary. The added blocks in the cited locations are multi-line rationale-heavy comments
explaining design decisions and test strategy in detail, which fits the checklist’s failure criteria
for overly verbose comments.

CLAUDE.md: Prefer self-explanatory code over verbose comments
npm/kcap/bin/kcap.test.js[111-120]
npm/kcap/bin/kcap.js[375-387]
src/Capacitor.Cli/Commands/ReportVersionCommand.cs[7-40]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Overly verbose comments were added that largely restate design rationale inline, contrary to the guideline to prefer self-explanatory code and concise comments.

## Issue Context
The following new comment blocks are long and descriptive (multi-paragraph), making the updated areas harder to scan and maintain. Consider shortening to a brief intent comment and letting naming/structure carry the rest, or moving deeper rationale to an ADR/design doc if needed.

## Fix Focus Areas
- npm/kcap/bin/kcap.test.js[111-120]
- npm/kcap/bin/kcap.js[375-387]
- src/Capacitor.Cli/Commands/ReportVersionCommand.cs[7-40]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Discovery exceeds time budget ✓ Resolved 🐞 Bug ☼ Reliability

Description

ReportVersionCommand applies its 5s timeout only to the final probe GET, but provider discovery
during CreateClientWithAuthStatusAsync has no command-level deadline because HandleAsync passes no
CancellationToken. This can block report-version until the npm wrapper kills it at 8s, making
kcap update slower than intended.

Code

src/Capacitor.Cli/Commands/ReportVersionCommand.cs[R46-49]

+            var (client, status) = await HttpClientExtensions.CreateClientWithAuthStatusAsync(baseUrl);
+
+            using (client) {
+                if (status is not (AuthStatus.Ok or AuthStatus.NoAuthRequired)) return 0;

Evidence

HandleAsync calls CreateClientWithAuthStatusAsync without providing a CancellationToken;
CreateClientWithAuthStatusAsync accepts a token and passes it into DiscoverProviderAsync;
DiscoverProviderAsync performs a GetAsync using that token, so with the default token there is no
command-level deadline on provider discovery.

src/Capacitor.Cli/Commands/ReportVersionCommand.cs[44-55]
src/Capacitor.Cli.Core/HttpClientExtensions.cs[44-50]
src/Capacitor.Cli.Core/HttpClientExtensions.cs[245-287]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`ReportVersionCommand` claims a bounded (~5s) best-effort request, but only the final `GetOnceAsync` is bounded. Provider discovery (`DiscoverProviderAsync`) is executed during client creation and can run without a deadline because `HandleAsync` calls `CreateClientWithAuthStatusAsync(baseUrl)` with default `CancellationToken`.

### Issue Context
`DiscoverProviderAsync` uses `HttpClient.GetAsync(..., ct)` and relies on the passed token to bound the call.

### Fix
- In `ReportVersionCommand.HandleAsync`, create a `CancellationTokenSource` with the desired total budget (e.g., 5s).
- Pass that token into `CreateClientWithAuthStatusAsync(effectiveBaseUrl, ct: budgetToken)`.
- Optionally also pass the same token into `GetOnceAsync(..., ct: budgetToken)` so cancellation is consistent.

### Fix Focus Areas
- src/Capacitor.Cli/Commands/ReportVersionCommand.cs[44-55]
- src/Capacitor.Cli.Core/HttpClientExtensions.cs[44-50]
- src/Capacitor.Cli.Core/HttpClientExtensions.cs[245-287]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. Fragile text-based test ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The new kcap.test.js unit test asserts an exact source substring for the `execFileSync(...
report-version ...)` call, coupling correctness to formatting/quoting. Small refactors (e.g.,
whitespace, quote changes, reformatting arguments) will break CI without behavior changes.

Code

npm/kcap/bin/kcap.test.js[R129-132]

+  const refreshIdx = runUpdateBody.indexOf("runRefreshes(");
+  const reportIdx  = runUpdateBody.indexOf('execFileSync(binaryPath, ["report-version", "--no-update-check"]');
+  const exitIdx    = runUpdateBody.lastIndexOf("process.exit(0)");
+

Evidence

The test reads kcap.js as text and uses indexOf on a single-quoted, formatting-specific string
for the execFileSync invocation, so formatting-only changes will fail the assertion.

npm/kcap/bin/kcap.test.js[111-146]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The new test validates behavior by searching for a hard-coded source substring, which is brittle (quote/whitespace/formatting changes break the test).

### Issue Context
The comment explains there is no easy seam to mock `execFileSync` because it’s destructured at module load time, so a source-shape assertion is understandable; the problem is the *exact* substring match is more brittle than necessary.

### Fix
Keep the general “source-shape” approach, but make it less formatting-coupled:
- Use a regex that tolerates whitespace and either quote style.
- Or assert separately that `runUpdateBody` contains `execFileSync(binaryPath` AND `report-version` AND `--no-update-check`, then keep the ordering checks.

### Fix Focus Areas
- npm/kcap/bin/kcap.test.js[111-146]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[realtonyyoung]

All qodo findings addressed in b4acedb:

1. [Sec] Wrong-host probe URL — now computes one effectiveBaseUrl with the same fallback chain as the client builder (incl. KCAP_URL), passed to both CreateClientWithAuthStatusAsync and the probe URL, so the authenticated request and the GET target can't diverge. Test added (null baseUrl + KCAP_URL ⇒ GET lands on the KCAP_URL host).
2. [Rule] Verbose comments — trimmed the ReportVersionCommand doc + the kcap.js/kcap.test.js blocks to concise form (code unchanged).
3. [Reliability] Discovery time budget — a single ~5s CancellationTokenSource now bounds the WHOLE command (provider discovery + the GET), threaded through both; still returns 0 on timeout. Test added (10s-delayed /auth/config ⇒ returns 0 within budget).
4. [Maint] Fragile source test — slimmed kcap.test.js to tolerant invariants (report-version present, ordered after refresh / before exit, inside try/catch) instead of exact string literals.