Skip to content

[AI-1283] Deterministic update notice + version/preference headers - #507

Open
realtonyyoung wants to merge 9 commits into
mainfrom
tony/ai-1283-cli-update-notice
Open

[AI-1283] Deterministic update notice + version/preference headers#507
realtonyyoung wants to merge 9 commits into
mainfrom
tony/ai-1283-cli-update-notice

Conversation

@realtonyyoung

Copy link
Copy Markdown
Collaborator

Part of AI-1283 — the kcap-cli half of "your kcap CLI is out of date" notifications. The kcap-server half (npm poller, per-user version registry, web banner, notification-centre kind, hook version field) already merged in kcap-server#1384. This PR makes the local update notice deterministic and starts transmitting the observation headers that server side consumes. Immediately useful on its own; the headers are inert until a server that reads them is deployed (and the server already ships that).

What changed

  • Deterministic exit notice — the existing npm update hint was fire-and-forget and only ever awaited inside Claude hooks (where nobody reads it). It now runs in a try/finally around the whole dispatch (covering --help and the no-server early exits) and prints after every human-facing command. Suppressed for agent-spawned/protocol/long-running commands (hook, generate-whats-done, set-title, copilot-finalize, mcp *, watch, the whole daemon family) and for update/uninstall/--no-update-check/update_check=false.
  • Cache + backoffUpdateCacheRecord adds failure/backoff fields (backward-compatible with the legacy two-field cache): a failed poll is retained with a 1h backoff so repeated quick commands don't re-hit the network, and the passive path runs under a bounded budget (300ms warm cache / 3s network with a 500ms cleanup grace) so a slow registry can't delay exit or livelock. kcap update/--check keep the 5s bound and bypass the backoff.
  • kcap status now prints the installed CLI version, with an inline (update available: X) when behind — sharing one check with the exit notice (no double print, at most one network call).
  • Observation headers — every authenticated request (via the CreateClientCoreAsync choke point, plus the whoami raw probe) carries X-Kcap-Cli-Version; X-Kcap-Update-Check: off is added only when the user has opted out. These let the server surface the banner / notification and honor the opt-out.
  • Opt-outkcap config set update_check false is now the full, persisted opt-out: it silences the local notice, the kcap status annotation, the transmitted headers (so the server-side banner/notification honor it), and the in-agent nudge. --no-update-check stays a one-shot local suppression.
  • README + help-update.txt + config help updated.

Notes

  • Built TDD, task-by-task, each reviewed (incl. mutation-testing the wire-contract headers and the notice-suppression gate).
  • No Linear IDs in any .cs file (repo lint).

🤖 Generated with Claude Code

…le-backoff reads

Cache schema (UpdateCacheRecord) adds attempted_at/failed alongside the
existing latest_version/checked_at, reading legacy two-field files as
plain success records. CheckForUpdateAsync's decision ladder is now:
fresh success record -> cached; failed record within a 1h backoff and
not forceCheck -> retained last-known version; otherwise fetch. The
fetch honors a passed CancellationToken so a future caller can bound
the passive path without stalling every CLI invocation; the cache
write itself never uses that token so a cancelled/failed fetch can
still persist its backoff record.

Part of AI-1283.
Replaces the fire-and-forget update-check Task.Run in Program.cs (only reliably
awaited inside a Claude hook, where nobody reads stderr) with UpdateNotice.FlushAsync,
run from a finally wrapping the whole command dispatch — including the --help and
no-server-configured early exits — so the notice fires deterministically for every
human-facing invocation. UpdateNotice.IsHumanFacing is the suppression predicate
(hooks/generators, mcp, watch, daemon run, update/uninstall, --no-update-check); a
shared lazily-started check task plus a MarkReported flag let a future exit-time
surface reuse the same result without a second network call or a double print.
Adds --no-update-check to every kcap-hook invocation in hooks.json belt-and-braces.
Adds a Version line to `kcap status` (kcap {current}), reusing
UpdateNotice's shared lazy update-check (GetSharedCheckAsync) so no
second network round-trip happens. When a newer version is available
it annotates inline (`kcap 0.11.12 (update available: 0.11.14)`) and
calls UpdateNotice.MarkReported() so Program.cs's exit-time footer
does not also print. Respects --no-update-check and a disabled
profile.UpdateCheck by skipping the check entirely rather than
suppressing an already-performed one.
Every client from HttpClientExtensions.CreateClientCoreAsync (the sole
choke point for authenticated CLI requests) and WhoamiCommand's raw probe
client now carry X-Kcap-Cli-Version. X-Kcap-Update-Check: off is added only
when the active profile has update_check disabled; its absence on a
version-carrying request is read by the server as "on".
…oadened opt-out

update_check=false now silences kcap's update nudging everywhere, not just
the local stderr hint: ClaudeHookCommand skips emitting the VersionNudgeEmitter
fragment into a Claude Code session's additionalContext when the active
profile has opted out, even if the server still sends a newer `version`.
help-update.txt, the config-set help text, and the README are updated to
describe the broadened meaning (stderr hint, transmitted version/preference
headers, in-agent nudge).
@linear-code

linear-code Bot commented Aug 9, 2026

Copy link
Copy Markdown

AI-1283

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Deterministic update notice + CLI version/opt-out headers on every request

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Replaces the fire-and-forget npm update-check with a deterministic try/finally
 (UpdateNotice.FlushAsync) that awaits before every human-facing command exits.
• Adds failure/backoff fields to the update-check cache and a bounded budget (300ms warm cache / 3s
 network) so a slow registry never stalls or livelocks exit.
• Attaches X-Kcap-Cli-Version and X-Kcap-Update-Check headers to every authenticated request so
 the server can drive its own update banner/notification and honor opt-outs.
• kcap status now prints the installed version with an inline "update available" annotation,
 sharing one check with the exit notice (no duplicate prints/network calls).
• Makes kcap config set update_check false the full, persisted opt-out across the local notice,
 status annotation, transmitted headers, and in-agent nudge.
• Updates README, help-update.txt, and config help text; adds extensive unit/integration test
 coverage.
Diagram

graph TD
    A["Program.cs dispatch"] -->|"finally"| B["UpdateNotice.FlushAsync"]
    B --> C["UpdateNotice.GetSharedCheckAsync"]
    C --> D["UpdateCommand.CheckForUpdateWithBudgetAsync"]
    D --> E[("Update cache file")]
    D --> F["npm registry"]
    G["StatusCommand.WriteVersionLineAsync"] --> C
    H["HttpClientExtensions.CreateClientCoreAsync"] --> I["AttachObservationHeadersAsync"] --> J["kcap-server"]
    K["WhoamiCommand.ProbeAsync"] --> I

    subgraph Legend
        direction LR
        _svc(["Process/Logic"]) ~~~ _db[("Cache/File")] ~~~ _ext{{"External service"}}
    end
Loading
High-Level Assessment

The PR's approach — a shared lazily-started check task guarded by a suppression predicate, plumbed through an existing single HTTP client choke point for headers — is the right design given the constraints (must never block exit, must not double-check, must be inert until server support exists). Alternatives like a background daemon-based checker or always-blocking network check were rightly rejected in favor of a cheap, budget-bounded, cache-backed approach that reuses existing infrastructure.

Files changed (20) +1389 / -94

Enhancement (7) +467 / -75
HttpClientExtensions.csAttach CLI version/update-check headers to every client +53/-0

Attach CLI version/update-check headers to every client

• Introduces CreateClientCoreAsync wrapper that attaches X-Kcap-Cli-Version and X-Kcap-Update-Check headers via new AttachObservationHeadersAsync, applied at the single choke point used by all authenticated requests.

src/Capacitor.Cli.Core/HttpClientExtensions.cs

ClaudeHookCommand.csRemove updateCheckTask plumbing; gate in-agent nudge on update_check +14/-16

Remove updateCheckTask plumbing; gate in-agent nudge on update_check

• Removes the now-unused updateCheckTask parameter threaded through hook handling, and skips emitting the in-agent version nudge fragment entirely when update_check is disabled.

src/Capacitor.Cli/Commands/ClaudeHookCommand.cs

StatusCommand.csAdd Version line with inline update-available annotation +57/-1

Add Version line with inline update-available annotation

• kcap status now prints the installed CLI version, reusing UpdateNotice's shared check to show an inline (update available: X) annotation without a duplicate network call or duplicate print.

src/Capacitor.Cli/Commands/StatusCommand.cs

UpdateCommand.csAdd cache backoff, budgeted passive check, and richer result type +215/-44

Add cache backoff, budgeted passive check, and richer result type

• Introduces UpdateCacheRecord with failure/backoff fields (backward compatible with legacy cache), a bounded two-tier budget (CheckForUpdateWithBudgetAsync) for the passive exit-time path, and refactors CheckForUpdateAsync to return a structured UpdateCheckResult while honoring cancellation for passive callers.

src/Capacitor.Cli/Commands/UpdateCommand.cs

WhoamiCommand.csAttach observation headers to whoami's raw probe request +5/-0

Attach observation headers to whoami's raw probe request

• Explicitly calls AttachObservationHeadersAsync on the WhoamiCommand probe client since it deliberately bypasses the shared client choke point.

src/Capacitor.Cli/Commands/WhoamiCommand.cs

Program.csWrap command dispatch in try/finally to flush update notice +14/-14

Wrap command dispatch in try/finally to flush update notice

• Replaces the old fire-and-forget Task.Run update check with a try/finally around the entire command dispatch that awaits UpdateNotice.FlushAsync on every exit path, including --help and no-server-configured early returns.

src/Capacitor.Cli/Program.cs

UpdateNotice.csNew UpdateNotice class: deterministic exit-time update hint +109/-0

New UpdateNotice class: deterministic exit-time update hint

• Adds the suppression predicate (IsHumanFacing), a shared lazily-started update check, a MarkReported gate to avoid double-printing, and FlushAsync as the single exit-time entry point that prints the update hint.

src/Capacitor.Cli/UpdateNotice.cs

Refactor (1) +4 / -2
CrashReporter.csExpose FailOpenCommands set as internal for reuse +4/-2

Expose FailOpenCommands set as internal for reuse

• Changes FailOpenCommands visibility to internal so UpdateNotice.IsHumanFacing can reuse the same agent-spawned command population.

src/Capacitor.Cli/CrashReporter.cs

Tests (8) +880 / -7
UpdateChannelQueryTests.csAdd cache/backoff/cancellation integration tests for update checks +134/-4

Add cache/backoff/cancellation integration tests for update checks

• Adds tests covering slow-but-completing responses caching correctly, passive-token cancellation triggering backoff, the 1h backoff policy, and forceCheck bypassing backoff.

test/Capacitor.Cli.Tests.Integration/UpdateChannelQueryTests.cs

UpdateNoticeDeliveryTests.csNew end-to-end tests for the exit-time update notice +277/-0

New end-to-end tests for the exit-time update notice

• Spawns the real compiled kcap binary with a seeded cache to verify the notice prints for human-facing commands, is suppressed for hooks/mcp/watch/opt-outs, and that kcap status single-reports instead of double-printing.

test/Capacitor.Cli.Tests.Integration/UpdateNoticeDeliveryTests.cs

ClaudeHookCommandTests.csAdd tests for update_check gating the in-agent nudge +48/-2

Add tests for update_check gating the in-agent nudge

• Adds paired tests proving the in-agent version nudge fragment is suppressed when update_check is off and still emitted when on.

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

UnusableUrlGuardTests.csRemove obsolete updateCheckTask argument from test call +0/-1

Remove obsolete updateCheckTask argument from test call

• Updates a HandleWithDeps call site to drop the removed updateCheckTask parameter.

test/Capacitor.Cli.Tests.Unit/Http/UnusableUrlGuardTests.cs

ObservationHeaderTests.csNew tests for CLI version/update-check wire headers +161/-0

New tests for CLI version/update-check wire headers

• Verifies the CreateClientCoreAsync choke point and the WhoamiCommand probe correctly attach or omit X-Kcap-Cli-Version and X-Kcap-Update-Check headers based on profile settings.

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

StatusVersionLineFormattingTests.csNew unit tests for status Version line formatting +43/-0

New unit tests for status Version line formatting

• Pure formatting tests for StatusCommand.FormatVersionLine covering null, not-newer, newer, and defensive null-Latest cases.

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

UpdateCacheRecordTests.csNew unit tests for UpdateCacheRecord freshness/backoff logic +107/-0

New unit tests for UpdateCacheRecord freshness/backoff logic

• Covers legacy cache compatibility, JSON round-tripping, and IsFresh/InFailureBackoff arithmetic.

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

UpdateNoticeIsHumanFacingTests.csNew unit tests for UpdateNotice.IsHumanFacing predicate +110/-0

New unit tests for UpdateNotice.IsHumanFacing predicate

• Exercises both suppressed (hooks, mcp, watch, daemon family, update/uninstall, --no-update-check) and non-suppressed command cases for the notice suppression predicate.

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

Documentation (3) +31 / -3
README.mdDocument deterministic update notice and opt-out behavior +20/-2

Document deterministic update notice and opt-out behavior

• Explains the new status Version line, the exit-time update notice, the transmitted version/opt-out headers, and how update_check vs --no-update-check differ.

README.md

help-update.txtDocument update_check config option scope +10/-0

Document update_check config option scope

• Adds help text explaining that update_check controls the stderr hint, server headers, and in-agent nudge.

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

ConfigCommand.csExpand update_check help description +1/-1

Expand update_check help description

• Updates the config set usage text for update_check to clarify it now controls the stderr hint, server headers, and in-agent nudge.

src/Capacitor.Cli/Commands/ConfigCommand.cs

Other (1) +7 / -7
hooks.jsonPass --no-update-check to all Claude hook invocations +7/-7

Pass --no-update-check to all Claude hook invocations

• Adds the --no-update-check flag to every 'kcap hook --claude' command so hooks never trigger or print the update notice.

kcap/hooks/hooks.json

@qodo-code-review

qodo-code-review Bot commented Aug 9, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Remediation recommended

1. Help text wrong on headers ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
help-update.txt claims update_check=false omits both X-Kcap-Cli-Version and
X-Kcap-Update-Check, but the implementation always sends the CLI version header when available and
sends X-Kcap-Update-Check: off when opted out. This misrepresents the opt-out’s wire behavior and
can confuse users/support expectations.
Code

src/Capacitor.Cli.Core/Resources/help-update.txt[R33-35]

+  human-facing commands; omits the X-Kcap-Cli-Version/X-Kcap-Update-Check
+  headers it would otherwise send with every server request (so your
+  server's own out-of-date banner and notification stay silent for you);
Evidence
The help text explicitly says the headers are omitted, but the implementation and tests show they
are still sent (version always when resolvable, plus an explicit opt-out marker when disabled).

src/Capacitor.Cli.Core/Resources/help-update.txt[30-38]
src/Capacitor.Cli.Core/HttpClientExtensions.cs[155-190]
test/Capacitor.Cli.Tests.Unit/ObservationHeaderTests.cs[50-74]
test/Capacitor.Cli.Tests.Unit/ObservationHeaderTests.cs[111-134]

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

### Issue description
`src/Capacitor.Cli.Core/Resources/help-update.txt` currently says that `update_check=false` *omits* the `X-Kcap-Cli-Version` / `X-Kcap-Update-Check` headers. That contradicts the actual behavior: the client sends `X-Kcap-Cli-Version` whenever a real display version is available, and it sends `X-Kcap-Update-Check: off` specifically to declare opt-out.

This is a user-facing contract description; it should match the actual wire behavior and the unit tests.

### Issue Context
- `HttpClientExtensions.AttachObservationHeadersAsync` always adds `X-Kcap-Cli-Version` unless the version is blank/`unknown`, and adds `X-Kcap-Update-Check: off` only when `profile.UpdateCheck == false`.
- `ObservationHeaderTests` asserts both headers are present on the `whoami` probe when `update_check` is off.

### Fix Focus Areas
- src/Capacitor.Cli.Core/Resources/help-update.txt[30-38]

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


2. Verbose WriteVersionLineAsync comment ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
StatusCommand.HandleAsync adds a large explanatory comment block describing control flow and
duplication avoidance. This should be shortened or replaced with small helper methods/names that
encode the intent directly.
Code

src/Capacitor.Cli/Commands/StatusCommand.cs[R17-20]

+        // Version — reuses UpdateNotice's shared lazy update-check (Capacitor.Cli.UpdateNotice
+        // .GetSharedCheckAsync) rather than calling UpdateCommand.CheckForUpdateAsync directly, so
+        // this line and the exit-time footer (UpdateNotice.FlushAsync) never trigger two network
+        // round-trips for one invocation. When this line prints the inline annotation it calls
Evidence
PR Compliance ID 4 disallows overly verbose comments when intent can be expressed via
naming/structure. The new Version-line comment block is long, details multiple behaviors, and could
be reduced to a brief summary plus clearer helper method names.

CLAUDE.md: Avoid verbose comments; prefer self-explanatory code
src/Capacitor.Cli/Commands/StatusCommand.cs[17-25]

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

## Issue description
A long comment block was added to explain the Version-line behavior in `kcap status`; it is verbose and risks drifting.

## Issue Context
Compliance prefers minimal comments and self-explanatory code.

## Fix Focus Areas
- src/Capacitor.Cli/Commands/StatusCommand.cs[17-25]

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


3. Verbose UpdateNotice doc comment ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The new UpdateNotice class includes a long narrative comment block that restates behavior already
expressed by method names and structure. This adds maintenance overhead and increases the chance the
comment drifts from actual behavior over time.
Code

src/Capacitor.Cli/UpdateNotice.cs[R6-9]

+/// <summary>
+/// Deterministic exit-time "update available" notice for human-facing invocations.
+///
+/// <para>Before this, the hint was printed by a fire-and-forget <c>Task.Run</c> launched near the
Evidence
PR Compliance ID 4 requires avoiding verbose comments in favor of self-explanatory code. The added
multi-paragraph documentation on UpdateNotice is lengthy, includes historical narrative, and
duplicates intent that can be conveyed via concise summaries and clear naming.

CLAUDE.md: Avoid verbose comments; prefer self-explanatory code
src/Capacitor.Cli/UpdateNotice.cs[6-26]

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

## Issue description
`src/Capacitor.Cli/UpdateNotice.cs` introduces an overly verbose multi-paragraph doc comment that explains historical context and detailed behavior already evident from code.

## Issue Context
Compliance requires preferring self-explanatory code over verbose comments to reduce drift and maintenance burden.

## Fix Focus Areas
- src/Capacitor.Cli/UpdateNotice.cs[6-26]

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


Grey Divider

Tip of the day
💡 Did you know, you can ask Qodo to dismiss a finding you disagree with, with your reason on record

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/Capacitor.Cli/UpdateNotice.cs Outdated
Comment thread src/Capacitor.Cli/Commands/StatusCommand.cs Outdated
Comment thread src/Capacitor.Cli.Core/Resources/help-update.txt Outdated
@realtonyyoung

Copy link
Copy Markdown
Collaborator Author

All three qodo findings addressed in a60c100 (docs/comments only, no logic change):

  1. Verbose UpdateNotice doc comment — trimmed the class XML doc to a concise summary (kept only the load-bearing one-liners).
  2. Verbose StatusCommand comment — trimmed the inline block to 1–2 lines; the helper/method names carry the intent.
  3. [Bug] help-text wire behavior — corrected help-update.txt (and reconciled the README callout): with update_check=false the CLI still sends X-Kcap-Cli-Version and adds X-Kcap-Update-Check: off — the opt-out is a signal carried on the wire (the server honors it and suppresses its own banner/notification), not an omission. The prior 'omits the headers' wording was inaccurate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant