[AI-1863] Cap the passive update notice + kcap status at the connected server's version

[realtonyyoung]

Context

The server half (kcap-server #1404, AI-1863) caps CLI-update recommendations at min(npm latest, server version) and emits an X-Kcap-Server-Version response header. This is the client half: the CLI now consumes that header so its own passive "update available" stderr notice and the kcap status version line never steer a user to a CLI newer than the server they talk to — manual tenant rollouts can trail npm by days, and a newer CLI risks protocol mismatch against the older server.

What changed

• ServerVersionStore — a durable, per-normalized-server-URL cache of the server version (one flat file per server in the config dir; best-effort, with an in-process write-dedup so the per-response hot path touches disk at most once per distinct value).
• ServerVersionCaptureHandler — the outermost DelegatingHandler on every authenticated client (the CreateClientCoreImplAsync choke point, the same one that attaches the request-side observation headers), reading X-Kcap-Server-Version from each response into the store. No extra requests; best-effort, never alters the response.
• UpdateAdvisoryResolver — caps the npm-latest target at min(npm, cached server version), only on the stable latest channel with a cached, stable server version present (beta deliberately rides ahead; an absent cache keeps today's behaviour — the cold-start doctrine). Recomputes "newer" against the capped target, so a user already at/ahead of their server is never nagged.
• UpdateNotice (passive exit-time stderr notice) and kcap status render the capped advisory: when capped, a pinned npm install -g @kurrent/kcap@<target> command and a (server version) marker (plain kcap update follows the dist-tag and would overshoot the cap). kcap update and --check stay UNCAPPED — they're explicit actions, not recommendations.

Surface D (the in-agent VersionNudgeEmitter) needs no change: the server omits the hook version field when the target is capped, so the emitter only ever receives an uncapped target (where plain kcap update is correct).

Tests

29 unit tests, all green locally:

• ServerVersionStoreTests (round-trip, normalization, per-server independence, overwrite, no-op guards);
• ServerVersionCaptureHandlerTests (header present → captured; absent → nothing);
• UpdateAdvisoryResolverTests (the cap truth table — passthrough when uncapped/beta/prerelease/absent, cap at min, user-ahead-not-nagged, plus the stable-release gate);
• StatusVersionLineFormattingTests (bare / uncapped / capped-with-marker rendering).

README + help-update.txt updated in the same PR (house rule).

Completes the client half of AI-1863.

🤖 Generated with Claude Code

[linear-code]

AI-1863

[qodo-code-review]

PR Summary by Qodo

Cap passive CLI update hints to the connected server version

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Capture connected server version from X-Kcap-Server-Version responses into a durable cache.
• Cap passive update recommendations to min(npm latest, server version) on stable channel only.
• Update kcap status and stderr notice copy, plus docs and unit test coverage.

Diagram

graph TD
  S["Kcap Server"] -->|"X-Kcap-Server-Version"| H["HttpClient"] --> C["ServerVersionCaptureHandler"] --> V["ServerVersionStore (disk)"] --> R["UpdateAdvisoryResolver"] --> N["UpdateNotice (stderr)"]
  R --> T["kcap status (Version line)"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Actively fetch server version via a dedicated endpoint

• ➕ Works even if the user hasn’t made recent API calls that return the header
• ➕ Could validate/refresh version on-demand for kcap status
• ➖ Adds extra network requests and failure modes to a previously passive path
• ➖ Requires endpoint contract and authentication behavior across server versions

2. Keep the cache in-memory only (no disk persistence)

• ➕ Simpler implementation; avoids filesystem edge cases
• ➕ No need to manage per-server cache files
• ➖ Cold-start on every process run; cap often unavailable in short-lived CLI usage
• ➖ Doesn’t help users unless they happen to make a version-header-bearing request first

Recommendation: The chosen approach (passive capture + durable per-server cache + capping only for stable latest) is the best balance: it avoids extra requests, survives across CLI invocations, and minimizes behavior changes for beta users and cold-start scenarios. The best-effort semantics and recomputation of Newer against the capped target are particularly appropriate for a passive notice.

Files changed (11) +486 / -33

Enhancement (5) +223 / -20

HttpClientExtensions.csAdd response handler to capture server version header +38/-3

Add response handler to capture server version header

• Wraps authenticated HttpClient construction with a new outermost 'ServerVersionCaptureHandler' to record 'X-Kcap-Server-Version' from final responses (after any retry). Adds a constant for the header name and introduces the handler implementation as best-effort, non-invasive capture.

src/Capacitor.Cli.Core/HttpClientExtensions.cs

ServerVersionStore.csIntroduce durable per-server server-version cache +88/-0

Introduce durable per-server server-version cache

• Adds 'ServerVersionStore' to persist last-seen server version per normalized server URL (hashed filename in config dir). Implements best-effort read/write, URL normalization, atomic write via temp file, and in-process write dedup to avoid per-response rewrites.

src/Capacitor.Cli.Core/ServerVersionStore.cs

StatusCommand.csCap 'kcap status' Version line update annotation +18/-12

Cap 'kcap status' Version line update annotation

• Routes the update check result through 'UpdateAdvisoryResolver' and formats the Version line based on the capped target. Adds explicit rendering for the '(server version)' marker when the target is capped, and prevents duplicate exit-time notice when already reported.

src/Capacitor.Cli/Commands/StatusCommand.cs

UpdateAdvisory.csAdd capped update advisory model + resolver +61/-0

Add capped update advisory model + resolver

• Introduces 'UpdateAdvisory' and 'UpdateAdvisoryResolver' to compute the effective passive update target. Caps only on the stable 'latest' channel with a cached stable server version, uses 'min(npm latest, server version)', and recomputes 'Newer' against the capped target to avoid downgrade nudges.

src/Capacitor.Cli/UpdateAdvisory.cs

UpdateNotice.csRender capped passive stderr update notice with pinned command +18/-5

Render capped passive stderr update notice with pinned command

• Applies the capped advisory when emitting the passive exit-time update hint. When capped, prints a pinned 'npm install -g @kurrent/kcap@<version>' command and adds the '(server version)' marker; otherwise keeps the existing 'kcap update' guidance.

src/Capacitor.Cli/UpdateNotice.cs

Tests (4) +252 / -12

ServerVersionCaptureHandlerTests.csTest capturing 'X-Kcap-Server-Version' into the store +43/-0

Test capturing 'X-Kcap-Server-Version' into the store

• Adds unit tests driving 'ServerVersionCaptureHandler' with a stub inner handler to validate that responses with the header are captured and responses without it do not modify the store.

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

ServerVersionStoreTests.csTest server version cache persistence, normalization, and overwrite behavior +73/-0

Test server version cache persistence, normalization, and overwrite behavior

• Adds unit tests for round-tripping, unknown server behavior, blank input no-ops, URL normalization (case and trailing slash), per-server independence, and overwriting on new observations.

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

StatusVersionLineFormattingTests.csUpdate version-line formatting tests for capped advisory +21/-12

Update version-line formatting tests for capped advisory

• Refactors tests to validate formatting against 'UpdateAdvisory' rather than raw update-check results. Adds coverage for the '(server version)' marker and defensive handling when 'Newer' is true but the target is null.

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

UpdateAdvisoryResolverTests.csAdd truth-table unit tests for capping logic +115/-0

Add truth-table unit tests for capping logic

• Adds comprehensive unit tests for capping rules (stable latest only, cached stable server only), passthrough conditions (beta, no cache, malformed/prerelease cache), 'min(npm, server)' behavior, and recomputation of 'Newer' to avoid downgrade prompts.

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

Documentation (2) +11 / -1

README.mdDocument server-capped update annotation in 'kcap status' +2/-1

Document server-capped update annotation in 'kcap status'

• Updates the README to clarify that the 'kcap status' Version line is capped to the connected server version when tenant rollout trails npm, and that this is explicitly marked.

README.md

help-update.txtExplain passive update cap behavior and pinned install guidance +9/-0

Explain passive update cap behavior and pinned install guidance

• Extends update help text to explain why passive update hints and 'kcap status' are capped to the connected server version, how capped recommendations are displayed, and why 'kcap update' remains uncapped (explicit action).

src/Capacitor.Cli.Core/Resources/help-update.txt

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. Wrong server cache key ✓ Resolved 🐞 Bug ≡ Correctness

Description

ServerVersionStore.Normalize() lowercases the entire URL and doesn’t canonicalize default ports, so
path-routed deployments (case-sensitive paths) can collide and cap against the wrong server, or
equivalent URLs (e.g., https://x vs https://x:443) won’t share a cache entry. This undermines the
“per-server cap” guarantee and can produce incorrect update advice.

Code

src/Capacitor.Cli.Core/ServerVersionStore.cs[R79-81]

+    /// <summary>Normalizes a server URL to a stable cache key: trimmed, no trailing slash,
+    /// lower-cased (scheme+host+port only — these carry no case-sensitive path).</summary>
+    internal static string Normalize(string serverUrl) => serverUrl.Trim().TrimEnd('/').ToLowerInvariant();

Evidence

The new cache key lowercases the full URL string, but existing server-identity canonicalization in
the repo explicitly states that URL paths are significant and case-sensitive and that
implicit/explicit default ports should converge. Using the current Normalize() therefore risks
collisions and misses.

src/Capacitor.Cli.Core/ServerVersionStore.cs[79-87]
src/Capacitor.Cli.Core/Auth/ServerIdentity.cs[3-31]

Agent prompt

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

## Issue description
`ServerVersionStore.Normalize()` currently lowercases the full URL string and only trims trailing slashes. This can conflate distinct servers in path-routed deployments (path is case-sensitive) and fails to converge equivalent spellings that should represent the same server identity (implicit vs explicit default ports).

## Issue Context
The repo already defines a canonicalization for “server identity” used for token binding, which explicitly treats path as significant and case-sensitive and converges default ports.

## Fix Focus Areas
- src/Capacitor.Cli.Core/ServerVersionStore.cs[79-86]
- src/Capacitor.Cli.Core/Auth/ServerIdentity.cs[3-31]

## Suggested fix
- Replace `ServerVersionStore.Normalize()` with a canonicalization consistent with `ServerIdentity.Canonicalize()` (scheme+host normalization and effective port convergence, preserve path casing, trim trailing slash).
- If canonicalization fails (unparseable URL), fall back to a conservative normalization (e.g., trim + trim trailing slash) without lowercasing the entire string.
- Add/extend unit tests to cover:
 - Same server: `https://host` == `https://host:443`
 - Different servers: `/TenantA` != `/tenanta` (path case sensitivity)
 - Query/fragment behavior (if those can appear, ensure they don’t collapse unexpectedly).

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

---

2. Build metadata in target ✓ Resolved 🐞 Bug ≡ Correctness

Description

UpdateAdvisoryResolver can return a capped Target that includes the server version verbatim, even
though the code treats +buildmetadata as ignorable for comparison/gating. If the cached server
version contains build metadata (e.g. 0.11.15+sha.abc), it will leak into kcap status and the
passive update notice (including the pinned npm install command), producing confusing output and
potentially a non-resolving install spec.

Code

src/Capacitor.Cli/UpdateAdvisory.cs[R43-46]

+        // min(npm latest, server version): the server caps only when it is strictly older than npm latest.
+        var capped = PrereleaseSemver.IsNewer(latest, cachedServerVersion);
+        var target = capped ? cachedServerVersion : latest;
+

Evidence

The resolver’s stable-release gate explicitly ignores build metadata, and SemVer comparison ignores
it too, but the resolver returns the original cached string as Target. Both UpdateNotice and
StatusCommand render Target directly, and the test suite explicitly treats 0.11.15+sha.abc as a
stable input.

src/Capacitor.Cli/UpdateAdvisory.cs[34-48]
src/Capacitor.Cli/UpdateNotice.cs[81-102]
src/Capacitor.Cli/Commands/StatusCommand.cs[140-152]
src/Capacitor.Cli.Core/CapacitorVersion.cs[17-37]
src/Capacitor.Cli.Core/PrereleaseSemver.cs[30-37]
test/Capacitor.Cli.Tests.Unit/UpdateAdvisoryResolverTests.cs[105-114]

Agent prompt

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

## Issue description
When the advisory is server-capped, `UpdateAdvisoryResolver.Resolve()` uses `cachedServerVersion` verbatim as the `Target`. However, build metadata is explicitly ignored by the stable-release gate and SemVer comparisons, and the repo generally strips build metadata from user-facing version strings.

## Issue Context
- `PrereleaseSemver` ignores build metadata for comparisons.
- `CapacitorVersion.Display()` exists specifically to strip `+buildmetadata` from user-facing output.
- The passive notice prints `advisory.Target` directly into an npm install command.

## Fix Focus Areas
- src/Capacitor.Cli/UpdateAdvisory.cs[34-48]
- src/Capacitor.Cli/UpdateNotice.cs[81-102]
- src/Capacitor.Cli/Commands/StatusCommand.cs[141-152]
- src/Capacitor.Cli.Core/CapacitorVersion.cs[17-37]
- src/Capacitor.Cli.Core/PrereleaseSemver.cs[30-37]

## Suggested fix
- When producing a capped target, strip build metadata for display/install purposes:
 - e.g. `var serverDisplay = CapacitorVersion.Display(cachedServerVersion);`
 - set `target = capped ? serverDisplay : latest;`
- Keep using the original value (or a parsed form) for comparisons if needed; comparisons already ignore `+...`.
- Consider also stripping/trimming when persisting the header in `ServerVersionStore.Set` (optional), but the minimum fix is to ensure `UpdateAdvisory.Target` is the display-safe version.
- Add a unit test to assert that a cached server version with `+buildmetadata` yields a Target without `+...` and that the notice/status output uses the stripped value.

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

---

3. Verbose Normalize XML comment ✓ Resolved 📘 Rule violation ⚙ Maintainability ⭐ New

Description

The new XML doc comment for ServerVersionStore.Normalize() is overly detailed and reads like
prose, which can reduce code readability and maintainability. Consider trimming to the essential
behavior and moving extended rationale to higher-level documentation or a short internal design
note.

Code

src/Capacitor.Cli.Core/ServerVersionStore.cs[R80-83]

+    /// <summary>Normalizes a server URL to a stable cache key using the repo's own server identity
+    /// (<see cref="ServerIdentity.Canonicalize"/>): scheme and host are lower-cased and an implicit vs
+    /// explicit default port converges, but the path is preserved case-sensitively — a path-routed
+    /// deployment (<c>/TenantA</c> vs <c>/tenanta</c>) is a DISTINCT server, and flattening its case

Evidence

PR Compliance ID 7 requires new/updated comments to be brief and necessary. The added XML summary
for Normalize() spans multiple lines of detailed rationale and edge-case explanation beyond what’s
typically needed inline.

CLAUDE.md: Keep comments concise; prefer self-explanatory code
src/Capacitor.Cli.Core/ServerVersionStore.cs[80-86]

Agent prompt

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

## Issue description
`ServerVersionStore.Normalize()` has a very verbose XML summary comment that could be shortened to keep in-code commentary concise and rely more on self-explanatory code/naming.

## Issue Context
Compliance prefers concise comments and intent encoded in code structure/naming.

## Fix Focus Areas
- src/Capacitor.Cli.Core/ServerVersionStore.cs[80-86]

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

---

Context

⚠️ Tickets: not configured — ticket URL found in PR but could not be fetched — check ticket provider credentials

Tip of the day

💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous review results

Review updated until commit dd5d441

Results up to commit 4d83e22 ⚖️ Balanced

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

1. Wrong server cache key ✓ Resolved 🐞 Bug ≡ Correctness

Description

ServerVersionStore.Normalize() lowercases the entire URL and doesn’t canonicalize default ports, so
path-routed deployments (case-sensitive paths) can collide and cap against the wrong server, or
equivalent URLs (e.g., https://x vs https://x:443) won’t share a cache entry. This undermines the
“per-server cap” guarantee and can produce incorrect update advice.

Code

src/Capacitor.Cli.Core/ServerVersionStore.cs[R79-81]

+    /// <summary>Normalizes a server URL to a stable cache key: trimmed, no trailing slash,
+    /// lower-cased (scheme+host+port only — these carry no case-sensitive path).</summary>
+    internal static string Normalize(string serverUrl) => serverUrl.Trim().TrimEnd('/').ToLowerInvariant();

Evidence

The new cache key lowercases the full URL string, but existing server-identity canonicalization in
the repo explicitly states that URL paths are significant and case-sensitive and that
implicit/explicit default ports should converge. Using the current Normalize() therefore risks
collisions and misses.

src/Capacitor.Cli.Core/ServerVersionStore.cs[79-87]
src/Capacitor.Cli.Core/Auth/ServerIdentity.cs[3-31]

Agent prompt

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

## Issue description
`ServerVersionStore.Normalize()` currently lowercases the full URL string and only trims trailing slashes. This can conflate distinct servers in path-routed deployments (path is case-sensitive) and fails to converge equivalent spellings that should represent the same server identity (implicit vs explicit default ports).

## Issue Context
The repo already defines a canonicalization for “server identity” used for token binding, which explicitly treats path as significant and case-sensitive and converges default ports.

## Fix Focus Areas
- src/Capacitor.Cli.Core/ServerVersionStore.cs[79-86]
- src/Capacitor.Cli.Core/Auth/ServerIdentity.cs[3-31]

## Suggested fix
- Replace `ServerVersionStore.Normalize()` with a canonicalization consistent with `ServerIdentity.Canonicalize()` (scheme+host normalization and effective port convergence, preserve path casing, trim trailing slash).
- If canonicalization fails (unparseable URL), fall back to a conservative normalization (e.g., trim + trim trailing slash) without lowercasing the entire string.
- Add/extend unit tests to cover:
 - Same server: `https://host` == `https://host:443`
 - Different servers: `/TenantA` != `/tenanta` (path case sensitivity)
 - Query/fragment behavior (if those can appear, ensure they don’t collapse unexpectedly).

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

---

2. Build metadata in target ✓ Resolved 🐞 Bug ≡ Correctness

Description

UpdateAdvisoryResolver can return a capped Target that includes the server version verbatim, even
though the code treats +buildmetadata as ignorable for comparison/gating. If the cached server
version contains build metadata (e.g. 0.11.15+sha.abc), it will leak into kcap status and the
passive update notice (including the pinned npm install command), producing confusing output and
potentially a non-resolving install spec.

Code

src/Capacitor.Cli/UpdateAdvisory.cs[R43-46]

+        // min(npm latest, server version): the server caps only when it is strictly older than npm latest.
+        var capped = PrereleaseSemver.IsNewer(latest, cachedServerVersion);
+        var target = capped ? cachedServerVersion : latest;
+

Evidence

The resolver’s stable-release gate explicitly ignores build metadata, and SemVer comparison ignores
it too, but the resolver returns the original cached string as Target. Both UpdateNotice and
StatusCommand render Target directly, and the test suite explicitly treats 0.11.15+sha.abc as a
stable input.

src/Capacitor.Cli/UpdateAdvisory.cs[34-48]
src/Capacitor.Cli/UpdateNotice.cs[81-102]
src/Capacitor.Cli/Commands/StatusCommand.cs[140-152]
src/Capacitor.Cli.Core/CapacitorVersion.cs[17-37]
src/Capacitor.Cli.Core/PrereleaseSemver.cs[30-37]
test/Capacitor.Cli.Tests.Unit/UpdateAdvisoryResolverTests.cs[105-114]

Agent prompt

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

## Issue description
When the advisory is server-capped, `UpdateAdvisoryResolver.Resolve()` uses `cachedServerVersion` verbatim as the `Target`. However, build metadata is explicitly ignored by the stable-release gate and SemVer comparisons, and the repo generally strips build metadata from user-facing version strings.

## Issue Context
- `PrereleaseSemver` ignores build metadata for comparisons.
- `CapacitorVersion.Display()` exists specifically to strip `+buildmetadata` from user-facing output.
- The passive notice prints `advisory.Target` directly into an npm install command.

## Fix Focus Areas
- src/Capacitor.Cli/UpdateAdvisory.cs[34-48]
- src/Capacitor.Cli/UpdateNotice.cs[81-102]
- src/Capacitor.Cli/Commands/StatusCommand.cs[141-152]
- src/Capacitor.Cli.Core/CapacitorVersion.cs[17-37]
- src/Capacitor.Cli.Core/PrereleaseSemver.cs[30-37]

## Suggested fix
- When producing a capped target, strip build metadata for display/install purposes:
 - e.g. `var serverDisplay = CapacitorVersion.Display(cachedServerVersion);`
 - set `target = capped ? serverDisplay : latest;`
- Keep using the original value (or a parsed form) for comparisons if needed; comparisons already ignore `+...`.
- Consider also stripping/trimming when persisting the header in `ServerVersionStore.Set` (optional), but the minimum fix is to ensure `UpdateAdvisory.Target` is the display-safe version.
- Add a unit test to assert that a cached server version with `+buildmetadata` yields a Target without `+...` and that the notice/status output uses the stripped value.

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

---

[realtonyyoung]

Both qodo findings addressed in 6373435:

1. Wrong server cache key — ServerVersionStore.Normalize now reuses the repo's own ServerIdentity.Canonicalize (scheme+host lower-cased, default ports converged, path preserved case-sensitively), with a conservative non-lowercasing fallback for inadmissible URLs. Removes the divergent second URL-identity function; path-routed tenants no longer collide and :443 converges with implicit.
2. Build metadata in target — the capped target is now run through CapacitorVersion.Display to strip +buildmetadata, so a MinVer commit-SHA server version can't leak into the (server version) copy or the pinned npm install -g @kurrent/kcap@<target> command.

Added tests: default-port convergence, path-case significance, and build-metadata stripping on a capped target.

[realtonyyoung]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 6373435

[realtonyyoung]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit dd5d441