Skip to content

Stop local setup when another kcap is at or below 1.3.0 - #1666

Merged
alexeyzimarev merged 3 commits into
mainfrom
alexeyzimarev/ai-3495-client-floor
Oct 19, 2026
Merged

alexeyzimarev merged 3 commits into
mainfrom
alexeyzimarev/ai-3495-client-floor

Conversation

@alexeyzimarev

@alexeyzimarev alexeyzimarev commented Oct 19, 2026 •

Copy link
Copy Markdown
Member

AI-3495 (Linear only; there is no GitHub issue)

What & why

A local installation needs every kcap that talks to it to be newer than 1.3.0, the last release without the local destination check. kcap setup --local asks the kcap on PATH and the desktop app's bundled one for their versions and stops with client_below_floor when either is 1.3.0 or older, or reports none. The watcher, daemon and desktop app also send X-Kcap-Cli-Version on hub negotiation, which the local server needs in order to refuse a hub client below the floor.

Ships with kurrent-io/kcap-server#2505. Merge this PR first: the server refuses a hub negotiation that names no version, and only clients built with this change name one.

Where to look

The floor is "newer than 1.3.0" rather than "at least X", so main's 1.3.1-alpha builds and whatever the next release is called are admitted without knowing its number. LocalClientFloor.NewestRefused and the server's ClientFloorMiddleware.NewestRefused must move together.

Verification

  • Preflight table: a copy below the floor, a copy reporting no version, a copy at the floor, the app's copy, and a reuse are each refused with the message the app shows; a copy above the floor passes.
  • Copy discovery against stub executables, including a PATH link to the app's kcap counted once, the running kcap skipped, and a real process printing its version.
  • Hub negotiation for the watcher, the daemon and the app captured by a stub server: each carries the version header. With the tagging removed, the watcher and daemon tests fail (received "").
  • dotnet publish src/Capacitor.Cli/Capacitor.Cli.csproj -c Release prints no IL2xxx/IL3xxx warnings.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Local edition setup checks kcap copies on PATH and in the desktop app. Setup is blocked if a copy is version 1.3.0 or older, or cannot report its version, with guidance on updating it.
    • Local servers reject clients that do not meet the minimum version requirement; their hooks and watcher do not record activity.
    • Connections report the CLI version and, when applicable, whether update checks are disabled.

The floor is "newer than 1.3.0", so prereleases toward the next release are admitted. Hub negotiation now carries the version header, which the local server needs to refuse an old watcher or daemon.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Oct 19, 2026

Copy link
Copy Markdown

AI-3495

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-19T17:57:45.129534Z 7ebb189 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Enforce the local installation client version floor

🐞 Bug fix ✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Refuse local setup when another discovered kcap is at or below 1.3.0 or reports no version.
• Send the CLI version during watcher, daemon, and desktop hub negotiation so the local server can
 enforce its floor.
• Document the requirement and test copy discovery, refusals, and negotiation headers.
Diagram

graph TD
  S["Local setup"] --> C["Other copies"] --> F{"Newer than 1.3.0?"}
  F -- No --> R["Setup refused"]
  F -- Yes --> P["Setup proceeds"]
  H["Hub clients"] --> V["Version header"] --> L["Local server"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Rely on server-side enforcement alone
  • ➕ Avoids discovering and launching other installed kcap copies during setup.
  • ➖ Users would learn about incompatible copies only when those copies attempt to connect.
  • ➖ Setup could appear successful while existing hooks or watchers cannot use the local installation.

Recommendation: Keep both setup preflight and server-side enforcement: preflight gives actionable feedback before installation, while the server guards clients that connect later. Keep the CLI and server NewestRefused values synchronized, and merge this client change before the server begins requiring versioned hub negotiations.

Files changed (24) +444 / -10

Enhancement (12) +121 / -3
ServerConnectionService.csIdentify the desktop app during hub negotiation +2/-0

Identify the desktop app during hub negotiation

• Adds the shared CLI version header to the desktop app's SignalR connection.

src/Capacitor.App/Services/ServerConnectionService.cs

HttpClientExtensions.csProvide a shared hub version-header helper +11/-0

Provide a shared hub version-header helper

• Exposes the process's CLI version and a helper for adding it to SignalR negotiation headers.

src/Capacitor.Cli.Core/HttpClientExtensions.cs

LocalClientFloor.csDefine the local client version floor +16/-0

Define the local client version floor

• Admits versions newer than 1.3.0, including later prereleases, and rejects unreadable versions.

src/Capacitor.Cli.Core/LocalEdition/LocalClientFloor.cs

LocalInstallFailureCode.csAdd a client-below-floor failure code +3/-1

Add a client-below-floor failure code

• Introduces the client_below_floor wire value for incompatible installed clients.

src/Capacitor.Cli.Core/LocalEdition/LocalInstallFailureCode.cs

ServerConnection.csIdentify the daemon during hub negotiation +1/-0

Identify the daemon during hub negotiation

• Tags daemon SignalR negotiations with the shared CLI version header.

src/Capacitor.Cli.Daemon/Services/ServerConnection.cs

WatchCommand.csIdentify the watcher during hub negotiation +1/-0

Identify the watcher during hub negotiation

• Tags watcher SignalR negotiations with the shared CLI version header.

src/Capacitor.Cli/Commands/WatchCommand.cs

ClientCopy.csRepresent a discovered kcap copy +4/-0

Represent a discovered kcap copy

• Records a copy's source, path, and reported version for preflight.

src/Capacitor.Cli/LocalEdition/ClientCopy.cs

ClientCopyKind.csDistinguish PATH and desktop app copies +6/-0

Distinguish PATH and desktop app copies

• Defines copy kinds used to give source-specific setup guidance.

src/Capacitor.Cli/LocalEdition/ClientCopyKind.cs

ILocalClientCopies.csAbstract local client discovery +6/-0

Abstract local client discovery

• Provides the preflight with a testable source of other installed kcap copies.

src/Capacitor.Cli/LocalEdition/ILocalClientCopies.cs

LocalClientCopies.csDiscover and query other installed kcap copies +65/-0

Discover and query other installed kcap copies

• Checks the PATH executable and macOS desktop bundles, skipping the running executable and deduplicating linked copies. Queries each copy's version with bounded process execution and treats failed or malformed responses as missing.

src/Capacitor.Cli/LocalEdition/LocalClientCopies.cs

LocalInstaller.csPass client discovery into installation preflight +4/-1

Pass client discovery into installation preflight

• Adds an optional copy source to the installer and supplies it when running preflight checks.

src/Capacitor.Cli/LocalEdition/LocalInstaller.cs

LocalInstallerFactory.csEnable copy discovery for production installs +2/-1

Enable copy discovery for production installs

• Constructs the current-process copy finder when creating a local installer.

src/Capacitor.Cli/LocalEdition/LocalInstallerFactory.cs

Bug fix (1) +19 / -3
LocalPreflight.csRefuse setup for incompatible kcap copies +19/-3

Refuse setup for incompatible kcap copies

• Checks discovered copies after existing preflight checks and refuses installation or reuse when a version is at or below the floor or unavailable. The refusal names the copy and gives source-specific remediation.

src/Capacitor.Cli/LocalEdition/LocalPreflight.cs

Tests (10) +303 / -4
HubTestHost.csAllow hub tests to inspect incoming requests +8/-1

Allow hub tests to inspect incoming requests

• Adds an optional request inspection callback to the test host.

test/Capacitor.App.Tests.Unit/HubTestHost.cs

LocalInstallOutcomeTests.csClassify the new refusal as permanent +1/-0

Classify the new refusal as permanent

• Verifies that the app treats client_below_floor as requiring user action rather than an immediate retry.

test/Capacitor.App.Tests.Unit/LocalInstallOutcomeTests.cs

ServerConnectionServiceTests.csVerify the desktop app's negotiation header +17/-0

Verify the desktop app's negotiation header

• Captures a hub negotiation and asserts that it carries the process's CLI version.

test/Capacitor.App.Tests.Unit/ServerConnectionServiceTests.cs

TerminalAdviceTests.csPreserve actionable client-floor messages +9/-0

Preserve actionable client-floor messages

• Checks that PATH, desktop app, and missing-version refusals reach the user unchanged.

test/Capacitor.App.Tests.Unit/TerminalAdviceTests.cs

LocalBundleManifestTests.csVerify the new failure-code wire value +2/-1

Verify the new failure-code wire value

• Updates the failure-code count and asserts that client_below_floor has a distinct wire representation.

test/Capacitor.Cli.Core.Tests.Unit/LocalEdition/LocalBundleManifestTests.cs

LocalClientFloorTests.csTest client-floor version boundaries +22/-0

Test client-floor version boundaries

• Covers newer releases and prereleases, the 1.3.0 boundary, older versions, and unreadable values.

test/Capacitor.Cli.Core.Tests.Unit/LocalEdition/LocalClientFloorTests.cs

ServerConnectionVersionHeaderTests.csVerify the daemon's negotiation header +54/-0

Verify the daemon's negotiation header

• Uses a local SignalR host to assert that daemon negotiation carries the CLI version.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/ServerConnectionVersionHeaderTests.cs

WatcherHubCredentialTests.csVerify the watcher's negotiation header +10/-0

Verify the watcher's negotiation header

• Asserts that watcher negotiation sends the process's CLI version.

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

LocalClientCopiesTests.csTest copy discovery and version probing +107/-0

Test copy discovery and version probing

• Covers PATH and app discovery, symlink deduplication, skipping the running copy, unreadable responses, caching, and a real version-printing process.

test/Capacitor.Cli.Tests.Unit/LocalEdition/LocalClientCopiesTests.cs

LocalPreflightTests.csTest client-floor setup decisions +73/-2

Test client-floor setup decisions

• Covers allowed versions and refusals for old or versionless copies, including reuse. Also verifies that an earlier platform refusal does not probe other copies.

test/Capacitor.Cli.Tests.Unit/LocalEdition/LocalPreflightTests.cs

Documentation (1) +1 / -0
README.mdDocument the local client version requirement +1/-0

Document the local client version requirement

• Explains which installed copies setup checks, how to remedy an incompatible copy, and that the local server also refuses older clients.

README.md

@qodo-code-review

qodo-code-review Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Hub connections undo update opt-outs ✓ Resolved
Description
TagCliVersion adds X-Kcap-Cli-Version to watcher, daemon, and app hub requests without also
sending X-Kcap-Update-Check: off when the active profile has opted out. In kcap-server,
authenticated hub negotiations therefore record the user's preference as on, which can re-enable
update notices and session-start nudges.
Code

src/Capacitor.Cli.Core/HttpClientExtensions.cs[R50-51]

+    public static void TagCliVersion(IDictionary<string, string> headers) {
+        if (CliVersion is { } version) headers[CliVersionHeader] = version;
Relevance

●●● Strong

Accepted precedent requires paired update headers when honoring opt-out preferences.

PR-#507

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR helper sends only the version, and all three hub clients call it. The existing HTTP handler
shows the paired-header convention; the server middleware interprets a missing opt-out header on any
authenticated version-bearing request as false, and the registry uses the newer observation to
replace the stored preference. The server's update evaluator and hook response then consult that
preference.

kcap-cli -> kcap-server
src/Capacitor.Cli.Core/HttpClientExtensions.cs[40-52]
src/Capacitor.Cli.Core/Http/ObservationHeaderHandler.cs[17-24]
src/Capacitor.App/Services/ServerConnectionService.cs[289-295]
src/Capacitor.Cli.Daemon/Services/ServerConnection.cs[252-258]
src/Capacitor.Cli/Commands/WatchCommand.cs[75-82]
External repo: kurrent-io/kcap-server, src/Capacitor.Server/CliUpdate/CliVersionObserverMiddleware.cs [50-78]
External repo: kurrent-io/kcap-server, src/Capacitor.Server.Services/CliUpdate/CliVersionRegistry.cs [212-217]
External repo: kurrent-io/kcap-server, src/Capacitor.Server.Services/CliUpdate/CliUpdateEvaluator.cs [83-99]
External repo: kurrent-io/kcap-server, src/Capacitor.Api.Public/Hooks/SessionHookHandlers.cs [534-541]

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

## Issue description
Version-tagged hub requests omit the active profile's update-check opt-out. kcap-server interprets that omission as an opt-in and updates the user's stored preference.
## Fix Focus Areas
- src/Capacitor.Cli.Core/HttpClientExtensions.cs[40-52]
- src/Capacitor.App/Services/ServerConnectionService.cs[289-295]
- src/Capacitor.Cli.Daemon/Services/ServerConnection.cs[252-258]
- src/Capacitor.Cli/Commands/WatchCommand.cs[75-82]
## Recommended Fix
Make the hub-header helper receive the resolved profile preference and send `X-Kcap-Update-Check: off` alongside the version when opted out. Pass that preference from each hub client, and test authenticated negotiation with update checks disabled against the server's observation contract.

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


2. An old copy blocks interrupted setup recovery ✓ Resolved
Description
LocalPreflight.Check calls CheckClients() even when Decide() finds a setup journal and returns
a recovery plan. If another kcap is old or cannot report a version, InstallAsync exits before the
lifecycle can roll back an incomplete setup or finish a pending commit.
Code

src/Capacitor.Cli/LocalEdition/LocalPreflight.cs[R26-27]

+        var plan = Decide(version, afterRecovery: false);
+        CheckClients();
Relevance

●●● Strong

Client probing before journal recovery can block rollback or roll-forward; recovery paths are
prioritized in recent precedents.

PR-#1622

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Check() applies the newly added client check to every plan, including Recover. The installer
invokes it before LocalLifecycle.RunAsync, which is the only path to journal recovery; that
recovery handles both rollback and roll-forward.

src/Capacitor.Cli/LocalEdition/LocalPreflight.cs[21-29]
src/Capacitor.Cli/LocalEdition/LocalPreflight.cs[39-46]
src/Capacitor.Cli/LocalEdition/LocalInstaller.cs[81-92]
src/Capacitor.Cli.Core/LocalEdition/LocalLifecycle.cs[11-17]
src/Capacitor.Cli/LocalEdition/LocalSetupRecovery.cs[20-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
An old or unreadable secondary kcap prevents `kcap setup --local` from recovering an interrupted setup journal, leaving rollback or commit unfinished.

## Fix Focus Areas
- src/Capacitor.Cli/LocalEdition/LocalPreflight.cs[21-33]
- src/Capacitor.Cli/LocalEdition/LocalInstaller.cs[81-115]

## Recommended Fix
Allow an initial recovery plan to reach the lifecycle without probing other clients. After journal recovery, enforce the client floor before any new installation or reuse proceeds; retain the existing preflight refusal for plans that do not require recovery.

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



Remediation recommended

3. Tests bypass TempDir filesystem helpers 📘 Rule violation ⚙ Maintainability
Description
LocalClientCopiesTests creates directories and symbolic links with Directory.CreateDirectory and
File.CreateSymbolicLink beneath Tmp.PathTo(...) instead of using the TempDir helper methods.
These test-created paths are therefore maintained through ad-hoc filesystem calls rather than the
helper's supported creation and cleanup API.
Code

test/Capacitor.Cli.Tests.Unit/LocalEdition/LocalClientCopiesTests.cs[R46-47]

+        Directory.CreateDirectory(Tmp.PathTo("bin"));
+        File.CreateSymbolicLink(Tmp.PathTo("bin", "kcap"), self);
Relevance

●●● Strong

Recent test-review precedents accept enforcing TempDir helper usage for direct filesystem mutations.

PR-#803
PR-#1602

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The changed tests use Tmp.PathTo(...) as the destination for direct directory and symbolic-link
mutations, which matches the checklist's prohibited direct filesystem operations beneath a TempDir
instance.

Rule 2767472: Use TempDir helper for all temporary filesystem state in tests
test/Capacitor.Cli.Tests.Unit/LocalEdition/LocalClientCopiesTests.cs[46-47]
test/Capacitor.Cli.Tests.Unit/LocalEdition/LocalClientCopiesTests.cs[59-60]

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 tests create temporary directories and symbolic links directly beneath `TempDir` paths, bypassing the required temporary filesystem helper abstraction.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Unit/LocalEdition/LocalClientCopiesTests.cs[46-47]
- test/Capacitor.Cli.Tests.Unit/LocalEdition/LocalClientCopiesTests.cs[59-60]

## Recommended Fix
Replace direct `Directory.CreateDirectory` and `File.CreateSymbolicLink` calls under `Tmp.Path`/`Tmp.PathTo(...)` with equivalent `TempDir` helper operations, adding a helper for symbolic links if the shared helper does not provide one.

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

Dismiss ↗ | View ↗


4. Setup's version probe can stall on an npm check ✓ Resolved
Description
VersionOf runs the other kcap as kcap --version and leaves out --no-update-check, so
UpdateNotice.IsHumanFacing counts it as human-facing. A kcap on PATH that the app didn't
bundle then runs the budgeted npm update check on its way out (up to about 3.5 s when the cache is
stale) and can write the update cache, all inside a preflight documented as 'No lock, no network, no
writes'.
Code

src/Capacitor.Cli/LocalEdition/LocalClientCopies.cs[R47-48]

+            result = runner.RunBoundedAsync(path, ["--version"], new(Timeout: VersionTimeout), OutputLimit, CancellationToken.None)
+                .GetAwaiter().GetResult();
Relevance

●●● Strong

Adding --no-update-check is a deterministic fix matching the documented no-network, no-write
preflight contract.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Program.cs awaits UpdateNotice.FlushAsync for every invocation, --version included. IsHumanFacing
returns false only for app-bundled copies, certain commands or an explicit --no-update-check.
FlushAsync then calls CheckForUpdateWithBudgetAsync, whose network deadline is 3 s plus 500 ms of
cleanup. The app's own version probe (KcapCli.VersionAsync) passes --no-update-check for this
reason. The preflight here runs on both Preflight() and InstallAsync.

src/Capacitor.Cli/UpdateNotice.cs[44-52]
src/Capacitor.Cli/UpdateNotice.cs[82-118]
src/Capacitor.Cli/Commands/UpdateCommand.cs[227-240]
src/Capacitor.App/Services/KcapCli.cs[151-158]
src/Capacitor.Cli/LocalEdition/LocalInstaller.cs[72-79]

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

## Issue description
`LocalClientCopies.VersionOf` spawns `kcap --version` without `--no-update-check`. A `PATH` kcap the app didn't bundle then runs its exit-time npm update check (up to about 3.5 s) and may write the update cache during a preflight documented as no-network, no-writes.

## Fix Focus Areas
- src/Capacitor.Cli/LocalEdition/LocalClientCopies.cs[47-48]

## Recommended Fix
Call `runner.RunBoundedAsync(path, ["--version", "--no-update-check"], ...)`, which matches `KcapCli.VersionAsync` in the app. `Program.cs`'s `--version` case ignores extra args, so older kcaps still print `kcap <version>`. Update the stub-based tests in LocalClientCopiesTests if they assert the exact args.

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


5. Subprocess tests oversubscribe the suite ✗ Dismissed
Description
LocalClientCopiesTests places ParallelLimiter<SubprocessLimit> only on
Reads_the_version_a_real_copy_prints instead of annotating the test class. The class contains a
real ProcessRunner subprocess test, so other subprocess-heavy tests in the class or suite are not
consistently coordinated through the shared limiter.
Code

test/Capacitor.Cli.Tests.Unit/LocalEdition/LocalClientCopiesTests.cs[R98-101]

+    [Test]
+    [ParallelLimiter<SubprocessLimit>]
+    public async Task Reads_the_version_a_real_copy_prints() {
+        Tmp.CreateExecutable("bin/kcap", "#!/bin/sh\necho \"kcap 1.2.0\"\n");
Relevance

●●● Strong

Recent precedents require class-level subprocess limiting whenever any test launches real child
processes.

PR-#1205
PR-#1159

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The changed test class includes a real ProcessRunner invocation, while the limiter is added only
to the individual test method rather than the class, contrary to the checklist's required
class-level scheduling constraint.

Rule 2808212: Use ParallelLimiter<SubprocessLimit> for test classes that spawn real subprocesses
test/Capacitor.Cli.Tests.Unit/LocalEdition/LocalClientCopiesTests.cs[8-10]
test/Capacitor.Cli.Tests.Unit/LocalEdition/LocalClientCopiesTests.cs[98-103]

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 test class starts a real subprocess but applies `ParallelLimiter<SubprocessLimit>` only to one test method, rather than participating at the class level as required for subprocess-heavy test classes.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Unit/LocalEdition/LocalClientCopiesTests.cs[8-10]
- test/Capacitor.Cli.Tests.Unit/LocalEdition/LocalClientCopiesTests.cs[98-103]

## Recommended Fix
Add `[ParallelLimiter<SubprocessLimit>]` to `LocalClientCopiesTests` and remove the method-level limiter attribute so the class consistently uses the shared subprocess limit.

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


Grey Divider

Context sources
❌ Tickets: failed to load — Ticket collection could not establish complete evidence
✅ Compliance rules (platform): 59 rules
✅ Cross-repo context — repo relationships
  Explored: repo: kurrent-io/kcap-server (branch: alexeyzimarev/ai-3495-client-floor, sha: 0e98c2bc) — View relationship
Review mode: Auto: 🧠 Deep: Cross-cutting client-floor logic spans discovery, preflight, and three hub negotiation paths.

Grey Divider

Tip of the day
💡 Did you know, you can import rules and skills only from the repositories your team follows

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment on lines +46 to +47
Directory.CreateDirectory(Tmp.PathTo("bin"));
File.CreateSymbolicLink(Tmp.PathTo("bin", "kcap"), self);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

3. Tests bypass tempdir filesystem helpers 📘 Rule violation ⚙ Maintainability

LocalClientCopiesTests creates directories and symbolic links with Directory.CreateDirectory and
File.CreateSymbolicLink beneath Tmp.PathTo(...) instead of using the TempDir helper methods.
These test-created paths are therefore maintained through ad-hoc filesystem calls rather than the
helper's supported creation and cleanup API.
Agent Prompt
## Issue description
The tests create temporary directories and symbolic links directly beneath `TempDir` paths, bypassing the required temporary filesystem helper abstraction.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Unit/LocalEdition/LocalClientCopiesTests.cs[46-47]
- test/Capacitor.Cli.Tests.Unit/LocalEdition/LocalClientCopiesTests.cs[59-60]

## Recommended Fix
Replace direct `Directory.CreateDirectory` and `File.CreateSymbolicLink` calls under `Tmp.Path`/`Tmp.PathTo(...)` with equivalent `TempDir` helper operations, adding a helper for symbolic links if the shared helper does not provide one.

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

Dismiss ↗ | View ↗

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Directories now come from Tmp.CreateDir. TempDir has no symlink helper, so File.CreateSymbolicLink stays.

Comment thread src/Capacitor.Cli/LocalEdition/LocalPreflight.cs Outdated
Comment thread src/Capacitor.Cli/LocalEdition/LocalClientCopies.cs Outdated
Comment thread src/Capacitor.Cli.Core/HttpClientExtensions.cs Outdated
@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: e4e0e5af-22d5-4235-b63e-f63cd61983a2
📥 Commits

Reviewing files that changed from the base of the PR and between 0375c34 and fa37c70.

📒 Files selected for processing: 2
  • src/Capacitor.Cli.Daemon/DaemonConfig.cs
  • src/Capacitor.Cli.Daemon/Services/ServerConnection.cs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

📝 Walkthrough

Hub connections send CLI version observation headers. Local installation discovers PATH and desktop-app CLI copies and refuses setup when a copy is at or below version 1.3.0 or cannot report a version.

Changes: Local client version compatibility

Layer / File(s) Summary
Hub observation headers
src/Capacitor.Cli.Core/HttpClientExtensions.cs, src/Capacitor.App/Services/ServerConnectionService.cs, src/Capacitor.Cli.Daemon/DaemonConfig.cs, src/Capacitor.Cli.Daemon/Services/ServerConnection.cs, src/Capacitor.Cli/Commands/WatchCommand.cs, test/Capacitor.App.Tests.Unit/HubTestHost.cs, test/Capacitor.App.Tests.Unit/ServerConnectionServiceTests.cs, test/Capacitor.Cli.Daemon.Tests.Unit/Services/ServerConnectionVersionHeaderTests.cs, test/Capacitor.Cli.Tests.Integration/WatcherHubCredentialTests.cs, test/Capacitor.Cli.Core.Tests.Unit/HttpClientExtensionsObservationHeadersTests.cs
The app, daemon, and watcher add CLI version headers to hub requests. They add the update-check opt-out header when update checks are disabled. Tests inspect negotiation headers.
Version floor and CLI copy discovery
src/Capacitor.Cli.Core/LocalEdition/LocalClientFloor.cs, src/Capacitor.Cli/LocalEdition/ClientCopy.cs, src/Capacitor.Cli/LocalEdition/ClientCopyKind.cs, src/Capacitor.Cli/LocalEdition/ILocalClientCopies.cs, src/Capacitor.Cli/LocalEdition/LocalClientCopies.cs, test/Capacitor.Cli.Core.Tests.Unit/LocalEdition/LocalClientFloorTests.cs, test/Capacitor.Cli.Tests.Unit/LocalEdition/LocalClientCopiesTests.cs
The shared floor admits versions above 1.3.0. Discovery checks desktop-app and PATH copies, excludes the current executable, deduplicates canonical paths, and records null for unreportable versions.
Local preflight enforcement
src/Capacitor.Cli.Core/LocalEdition/LocalInstallFailureCode.cs, src/Capacitor.Cli/LocalEdition/LocalInstaller.cs, src/Capacitor.Cli/LocalEdition/LocalInstallerFactory.cs, src/Capacitor.Cli/LocalEdition/LocalPreflight.cs, test/Capacitor.Cli.Tests.Unit/LocalEdition/LocalPreflightTests.cs, test/Capacitor.App.Tests.Unit/LocalInstallOutcomeTests.cs, test/Capacitor.App.Tests.Unit/TerminalAdviceTests.cs, test/Capacitor.Cli.Core.Tests.Unit/LocalEdition/LocalBundleManifestTests.cs, README.md
The installer passes discovered copies to preflight. Preflight refuses setup for a copy that does not meet the version floor, but defers the check during recovery. The failure code, messages, tests, and README cover this requirement.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant LocalInstaller
  participant LocalPreflight
  participant LocalClientCopies
  participant KcapCopy
  participant LocalClientFloor
  LocalInstaller->>LocalPreflight: Run Check
  LocalPreflight->>LocalClientCopies: Find CLI copies
  loop Each discovered copy
    LocalClientCopies->>KcapCopy: Run --version --no-update-check
    KcapCopy-->>LocalClientCopies: Return version output or failure
  end
  LocalClientCopies-->>LocalPreflight: Return copies and versions
  LocalPreflight->>LocalClientFloor: Check version with Admits
  LocalPreflight-->>LocalInstaller: Return plan or ClientBelowFloor
Loading

Suggested reviewers: realtonyyoung

Merge Risk: ⚪ Minimal · up to fa37c

The daemon carries the active profile’s update-check preference into hub negotiation, and supported builds are version-stamped. No concrete user-impacting regression is established, so the change appears mergeable under normal checks; server-side floor parity remains unconfirmed.

Security architecture risk: 🔵 Low · up to fa37c

The inspected changes preserve existing credentials and destination controls while adding compatibility checks. Remaining risk concerns coordination with the local server: matching version rules and mixed-version rollout behavior were not available to verify.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • observed — The new local execution surface is version probing of PATH-selected and installed-app executables. Production probing launches the selected file directly with separate arguments and the inherited process environment; it adds no sandbox or separate OS identity. Consequently, executable integrity remains a prerequisite, and version checking does not protect setup from a malicious replacement binary.

Trust Boundaries and Controls

  • observed — The changed daemon negotiation still targets the configured server's sessions hub and obtains its bearer through the existing credential source. Existing local-destination checks, redirect guarding and WebSocket bearer handling remain around the added metadata call. The inspected change does not replace these controls with version-based identity.

Resilience and Maintainability Implications

  • observed — The new refusal does not prevent interrupted setup recovery: initial preflight defers the copy check when a journal requires recovery. Subsequent install or reuse checks run under lifecycle and unit locks, before new setup work, and release the unit lock on refusal. Recovery preserves an already committed roll-forward rather than undoing it because of incompatible copies; cancellation after committed recovery can report that commit without a further reuse pass.
Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 24.62% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 65 functions across 25 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: local setup stops when another kcap is at or below version 1.3.0.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Oct 19, 2026
A server reads a version without the opt-out as the update check being on. An old kcap no longer blocks recovery of an interrupted setup; it is refused once recovery has run.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
coderabbitai[bot]
coderabbitai Bot previously approved these changes Oct 19, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@alexeyzimarev
alexeyzimarev merged commit 4efcc5d into main Oct 19, 2026
16 of 17 checks passed
@alexeyzimarev
alexeyzimarev deleted the alexeyzimarev/ai-3495-client-floor branch October 19, 2026 20:30
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