Let the browser ask this machine to fix a broken PATH

[George-Payne]

AI-2224

What & why

The Agents screen can raise a PATH warning — the login shell cannot find kcap, so hooks install, run and record nothing — and kcap daemon shim ensure can fix it, but nothing joins them. This gives the poll a request lane: the browser records a request, kcap setup performs it mid-wait and reports the outcome back, and the screen renders it.

The alternative was deferring the fix to the terminal after the poll ends. Rejected because it cannot report: the user leaves the browser, an admin prompt appears later in a terminal they have stopped watching, and a cancelled or a conflict row never reaches them. That is the same silent failure the warning is spending an error colour on.

The create body also gains platform, so the screen offers the button only for an explicit macos — the same shape as the warning, which appears only for an explicit login_shell_finds_cli: false.

Where to look

• FirstRunMachineActionOutcomes/Reasons are now the canonical token list and ShimEnsureJson uses them, so the verb's output and the browser's copy cannot drift apart.
• What does not cross: the report is two closed-set tokens and the request's timestamp. Detail is raw shell stderr and SudoFallback a composed sudo line; both stay in the terminal.
• ShimEnsureJson.ExitCode is derived and [JsonIgnore]d — the record is the --json document.

Verification

Capacitor.Cli.Core.Tests.Unit 2352/2352, Capacitor.Cli.Tests.Unit 3574/3574, Release AOT publish with no IL warnings.

Four guards are mutation-proven — dropping each one reddens tests: perform-once (2 fail), the echoed timestamp (2), performing before the finished test (1), the closed capability set (1).

[linear-code]

AI-2224

[qodo-code-review]

PR Summary by Qodo

Enable browser-requested PATH repair during setup

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Lets macOS browser setup request and receive outcomes for the PATH shim fix.
• Restricts machine actions to closed capability, outcome, reason, and timestamp tokens.
• Retries failed reports without repeating privileged actions, backed by comprehensive unit
 coverage.

Diagram

sequenceDiagram
    actor User
    participant Browser as Agents Screen
    participant Server as First-Run API
    participant Flow as Poll Loop
    participant Host as Setup Actions
    participant Shim as Shim Evaluator
    User->>Browser: Request PATH fix
    Browser->>Server: Store capability request
    Flow->>Server: Poll outstanding request
    Server-->>Flow: Capability and timestamp
    Flow->>Host: Perform named capability
    Host->>Shim: Evaluate PATH shim
    Shim-->>Host: Outcome tokens
    Host-->>Flow: Action result
    Flow->>Server: Report timestamped outcome
    Server-->>Browser: Render final outcome

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Defer repair to the terminal

• ➕ Keeps privileged work outside the polling loop
• ➕ Requires no machine-action request route
• ➖ Admin prompts can appear after the user leaves the terminal
• ➖ Cancellation, conflict, and failure outcomes cannot reach the waiting browser

2. Send commands or paths from the server

• ➕ Supports arbitrary future actions without new CLI capability mappings
• ➖ Expands the remote-command security surface
• ➖ Allows server-provided data to influence shell execution
• ➖ Weakens compatibility and validation guarantees

3. Run actions in a background worker

• ➕ Keeps browser polling responsive during the admin prompt
• ➖ Adds lifecycle, synchronization, and shutdown complexity
• ➖ Makes action completion and final-flow ordering harder to guarantee

Recommendation: Keep the PR's synchronous, closed-capability request lane. Reusing the existing shim evaluator prevents behavior drift, timestamp identity prevents stale reports, and separate performed/reported tracking gives the required privileged-action safety without introducing a background execution subsystem.

Files changed (17) +903 / -65

Enhancement (8) +357 / -12

BrowserFirstRunFlow.csExecute and reliably report polled machine actions +73/-5

Execute and reliably report polled machine actions

• Adds an optional machine-action host to the browser flow and processes requests before checking flow completion. Tracks performed and reported requests separately so privileged work runs once while failed reports retry against the original timestamp.

src/Capacitor.Cli.Core/FirstRun/BrowserFirstRunFlow.cs

FirstRunFlowClient.csAdd machine-action reporting API client +37/-2

Add machine-action reporting API client

• Extends the first-run channel with the action-report route and a recorded-status result. Also includes the machine platform in flow creation payloads.

src/Capacitor.Cli.Core/FirstRun/FirstRunFlowClient.cs

FirstRunFlowModels.csDefine platform and machine-action wire models +51/-0

Define platform and machine-action wire models

• Adds platform reporting, outstanding action responses, and timestamped action outcome requests. The report contract intentionally carries only closed tokens and excludes terminal-only error details.

src/Capacitor.Cli.Core/FirstRun/FirstRunFlowModels.cs

FirstRunFlowOutcomes.csValidate and map outstanding machine actions +32/-0

Validate and map outstanding machine actions

• Extracts only known, timestamped capabilities from poll responses and deduplicates repeated capability entries. Unknown or unidentifiable requests are ignored rather than executed.

src/Capacitor.Cli.Core/FirstRun/FirstRunFlowOutcomes.cs

FirstRunFlowProgress.csExpose pre-action progress notification +9/-0

Expose pre-action progress notification

• Adds a progress callback that warns the terminal before a browser-requested machine action begins, ensuring the macOS password prompt is expected.

src/Capacitor.Cli.Core/FirstRun/FirstRunFlowProgress.cs

FirstRunMachineActions.csIntroduce closed machine-action contracts +111/-0

Introduce closed machine-action contracts

• Defines supported platforms, the PATH shim capability, canonical outcome and refusal tokens, request identity, action results, and the host execution interface.

src/Capacitor.Cli.Core/FirstRun/FirstRunMachineActions.cs

FirstRunMachineReport.csReport the machine platform during first run +12/-4

Report the machine platform during first run

• Carries the detected platform through machine report evaluation and current-host discovery. This lets the browser expose PATH repair only for explicit macOS reports.

src/Capacitor.Cli.Core/FirstRun/FirstRunMachineReport.cs

SetupCommand.csWire PATH repair into interactive setup +32/-1

Wire PATH repair into interactive setup

• Adds the setup machine-action host for the PATH shim and injects it into the browser flow. Displays a terminal warning before the expected macOS administrator prompt.

src/Capacitor.Cli/Commands/SetupCommand.cs

Refactor (2) +88 / -45

DaemonShimCommands.csShare the PATH shim evaluator with browser setup +69/-37

Share the PATH shim evaluator with browser setup

• Separates silent shim evaluation from console reporting so setup and the command use the same decision ladder. Replaces duplicated string literals with canonical tokens and propagates cancellation through probing and installation.

src/Capacitor.Cli/Commands/DaemonShimCommands.cs

ShimEnsureJson.csDerive shim exit codes from canonical outcomes +19/-8

Derive shim exit codes from canonical outcomes

• Uses shared first-run outcome tokens and derives the process exit code from them. Keeps the derived exit code out of the JSON document and terminal-only details out of browser reports.

src/Capacitor.Cli/Commands/ShimEnsureJson.cs

Tests (5) +455 / -8

BrowserFirstRunFlowTests.csCover machine-action execution and retry guards +220/-4

Cover machine-action execution and retry guards

• Tests timestamp echoing, warning order, capability filtering, perform-once behavior, report retries, fresh requests, exception mapping, final-tick execution, and hosts without actions.

test/Capacitor.Cli.Core.Tests.Unit/FirstRun/BrowserFirstRunFlowTests.cs

FirstRunFlowClientTests.csVerify platform and action-report HTTP contracts +79/-2

Verify platform and action-report HTTP contracts

• Checks platform serialization, action endpoint payloads, omission of free-text details, and recorded-status handling for unsuccessful responses.

test/Capacitor.Cli.Core.Tests.Unit/FirstRun/FirstRunFlowClientTests.cs

FirstRunFlowOutcomesTests.csTest closed-set machine-action parsing +53/-0

Test closed-set machine-action parsing

• Verifies known requests are accepted while unknown capabilities, missing timestamps, duplicates, and absent action lists are handled safely.

test/Capacitor.Cli.Core.Tests.Unit/FirstRun/FirstRunFlowOutcomesTests.cs

FirstRunMachineReportTests.csTest platform reporting and detection +23/-2

Test platform reporting and detection

• Covers unchanged platform propagation and confirms the current host maps to one of the browser-supported platform tokens.

test/Capacitor.Cli.Core.Tests.Unit/FirstRun/FirstRunMachineReportTests.cs

DaemonShimCommandsTests.csTest silent shared shim evaluation +80/-0

Test silent shared shim evaluation

• Verifies browser-driven evaluation emits no console output, preserves refusal semantics, derives exit codes correctly, omits exit codes from JSON, and advertises only the PATH shim capability.

test/Capacitor.Cli.Tests.Unit/Commands/DaemonShimCommandsTests.cs

Documentation (1) +2 / -0

README.mdDocument browser-triggered PATH repair +2/-0

Document browser-triggered PATH repair

• Explains that the Agents screen can request the macOS PATH shim while setup waits, including the admin prompt, outcome reporting, and restricted capability boundary.

README.md

Other (1) +1 / -0

Models.csRegister action report JSON metadata +1/-0

Register action report JSON metadata

• Adds the machine-action report DTO to the source-generated JSON serialization context for AOT-safe HTTP payload serialization.

src/Capacitor.Cli.Core/Models.cs

[qodo-code-review]

Code Review by Qodo

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

1. CreateAsync comment restates test ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

The added comment repeats the test name and the immediately visible macos input/assertion rather
than documenting a non-obvious precondition or trap. It adds maintenance surface without information
needed to understand the test.

Code

test/Capacitor.Cli.Core.Tests.Unit/FirstRun/FirstRunFlowClientTests.cs[R98-99]

+    // What decides whether the screen offers to fix a broken PATH at all: the shim is macOS-only, so
+    // the browser draws its button for an explicit macos and nothing else.

Evidence

PR Compliance ID 24 identifies comments that restate code as failures. The cited comment states that
macos controls the PATH-fix affordance, while the adjacent test name and assertion directly
express that same behavior.

CLAUDE.md: Keep Comments Sparse, Current, and Focused on Live Constraints
test/Capacitor.Cli.Core.Tests.Unit/FirstRun/FirstRunFlowClientTests.cs[98-99]

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 comment above the platform serialization test restates the behavior already expressed by the test name and assertions.

## Issue Context
PR Compliance ID 24 requires sparse comments that capture non-obvious live constraints; this test is self-explanatory without the comment.

## Fix Focus Areas
- test/Capacitor.Cli.Core.Tests.Unit/FirstRun/FirstRunFlowClientTests.cs[98-99]

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

---

2. MachineActions comment references plans ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

The new XML comment explains the collection shape by saying a second capability is not planned,
which is planning history rather than a live source constraint. This context will become stale
independently of the wire-compatibility requirement the comment should document.

Code

src/Capacitor.Cli.Core/FirstRun/FirstRunFlowModels.cs[R159-161]

+    /// <para><b>A list because a second capability must not be a wire break</b>, not because one is
+    /// planned. Entries this build cannot act on are simply left alone: the request stays outstanding and
+    /// the browser goes on saying so, which is the honest state for a CLI too old to perform it.</para>

Evidence

PR Compliance ID 24 forbids comments that reference plans and requires comments to focus on current
constraints. The added paragraph explicitly says a second capability is not planned.

CLAUDE.md: Keep Comments Sparse, Current, and Focused on Live Constraints
src/Capacitor.Cli.Core/FirstRun/FirstRunFlowModels.cs[159-161]

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 `MachineActions` XML comment refers to whether another capability is planned, which violates the requirement that comments remain understandable from current source and avoid planning references.

## Issue Context
The durable constraint is that the wire uses a list for forward compatibility and unknown entries remain outstanding. Preserve that live compatibility rationale without discussing plans.

## Fix Focus Areas
- src/Capacitor.Cli.Core/FirstRun/FirstRunFlowModels.cs[159-161]

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

---

3. Cancellation swallowed, flow reports Finished ✓ Resolved 🐞 Bug ☼ Reliability

Description

In PerformRequestedAsync, an OperationCanceledException triggered by the caller's cancellation token
is caught and the method simply returns instead of propagating; control then resumes in PollAsync
and proceeds to the IsFinished check, so a canceled setup can still resolve as
FirstRunFlowResult.Finished instead of surfacing cancellation. This also silently drops the
in-flight action: it is never recorded in performed nor reported, so the browser sees the request
stay outstanding forever even though it was actually interrupted client-side.

Code

src/Capacitor.Cli.Core/FirstRun/BrowserFirstRunFlow.cs[R230-239]

+                try {
+                    result = await actions.PerformAsync(request.Capability, ct);
+                } catch (OperationCanceledException) when (ct.IsCancellationRequested) {
+                    // The caller is going away; reporting into a cancelled leg would be reporting to nobody.
+                    return;
+                } catch (Exception) {
+                    // `failed` rather than a refusal: something was attempted. A screen left waiting on an
+                    // outcome that threw is the state this lane exists to avoid.
+                    result = new FirstRunMachineActionResult(FirstRunMachineActionOutcomes.Failed, null);
+                }

Evidence

PerformRequestedAsync is invoked at line 170 before the IsFinished check at line 172; when
actions.PerformAsync(request.Capability, ct) throws OperationCanceledException with
ct.IsCancellationRequested true, the catch at lines 232-234 returns without recording or reporting,
and execution falls through to the IsFinished(last!) check at line 172, which can return Finished
despite the cancellation. Ordinary poll-loop cancellation instead propagates via Task.Delay(slice,
_clock, ct) at line 285 and is explicitly excluded from being treated as transient at
FirstRunFlowClient.cs IsTransient (lines 147-150), showing this swallow-and-continue behavior is
inconsistent with how cancellation is handled everywhere else in the same flow.

src/Capacitor.Cli.Core/FirstRun/BrowserFirstRunFlow.cs[164-172]
src/Capacitor.Cli.Core/FirstRun/BrowserFirstRunFlow.cs[211-239]
src/Capacitor.Cli.Core/FirstRun/BrowserFirstRunFlow.cs[274-290]
src/Capacitor.Cli.Core/FirstRun/FirstRunFlowClient.cs[144-150]

Agent prompt

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

## Issue description
In `PerformRequestedAsync`, when the caller's `CancellationToken` fires while `actions.PerformAsync(...)` is awaiting (e.g. the user cancels `kcap setup` while an admin password prompt is up), the `catch (OperationCanceledException) when (ct.IsCancellationRequested)` block just `return`s from the helper. Execution then resumes in the caller (`PollAsync`) and proceeds straight to the `IsFinished` check, meaning a canceled operation can still resolve as `FirstRunFlowResult.Finished` instead of surfacing the cancellation to `RunAsync`'s caller. The in-flight request is also left neither recorded nor reported, which is inconsistent with the rest of the flow's cancellation handling (e.g. `Task.Delay(slice, _clock, ct)` in `WaitForIntervalAsync`, and `IsTransient` in `FirstRunFlowClient` which explicitly excludes real caller cancellation from being treated as a retryable blip).

## Issue Context
`PerformRequestedAsync` is called from `PollAsync` right before the `IsFinished` check on every successful poll iteration. If it swallows cancellation instead of rethrowing/propagating it, `PollAsync` (and thus `RunAsync`) can return a `Finished`/normal result for a leg that was actually torn down mid-way, misleading the caller (e.g. `SetupCommand`) about the true outcome.

## Fix Focus Areas
- src/Capacitor.Cli.Core/FirstRun/BrowserFirstRunFlow.cs[230-239]
- src/Capacitor.Cli.Core/FirstRun/BrowserFirstRunFlow.cs[167-172]

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

---

4. Failed action report unretried on terminal poll ✓ Resolved 🐞 Bug ☼ Reliability

Description

When a poll response both finishes the flow and carries a machine-action request, a failed report
POST leaves the successfully performed request out of reported, but PollAsync immediately
returns Finished without another iteration to retry it. The action outcome can therefore be
permanently and silently lost on a transient non-2xx response or transport failure, defeating the
result-reporting path while the action itself remains marked as performed.

Code

src/Capacitor.Cli.Core/FirstRun/BrowserFirstRunFlow.cs[R167-172]

+                    // Before the finished test, so a request made on the last screen is not abandoned by a
+                    // flow that settles in the same tick — and the finished test then runs again below,
+                    // because the budget can pass while the user answers an admin prompt.
+                    await PerformRequestedAsync(serverUrl, flowId, last!, performed, reported, ct);
+
                    if (FirstRunFlowOutcomes.IsFinished(last!)) return new FirstRunFlowResult.Finished(last!);

Evidence

The comment at lines 167-169 says the finished test “runs again below” to accommodate a request made
near the end, but this only helps while the loop continues; when IsFinished(last!) is true at line
172, the method returns immediately. Reports are considered complete only when outcome.Recorded is
true and the request is added to reported at lines 245-257, while transport failures and non-2xx
responses leave it unreported; because IsFinished depends only on flow gates and steps, a terminal
response may still carry an action whose failed report then receives no retry. The existing test
A_report_that_did_not_land_is_retried_without_performing_again at
test/Capacitor.Cli.Core.Tests.Unit/FirstRun/BrowserFirstRunFlowTests.cs:721-732 demonstrates
retrying across two non-terminal poll responses, but there is no equivalent coverage or code path
for a report failure on the response that also finishes the flow.

src/Capacitor.Cli.Core/FirstRun/BrowserFirstRunFlow.cs[153-172]
src/Capacitor.Cli.Core/FirstRun/BrowserFirstRunFlow.cs[245-258]
src/Capacitor.Cli.Core/FirstRun/FirstRunFlowClient.cs[16-22]
test/Capacitor.Cli.Core.Tests.Unit/FirstRun/BrowserFirstRunFlowTests.cs[720-732]
src/Capacitor.Cli.Core/FirstRun/BrowserFirstRunFlow.cs[167-172]
src/Capacitor.Cli.Core/FirstRun/BrowserFirstRunFlow.cs[245-257]
src/Capacitor.Cli.Core/FirstRun/FirstRunFlowClient.cs[16-21]
src/Capacitor.Cli.Core/FirstRun/FirstRunFlowClient.cs[112-117]
src/Capacitor.Cli.Core/FirstRun/FirstRunFlowOutcomes.cs[98-109]

Agent prompt

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

## Issue description

`PollAsync` calls `PerformRequestedAsync(...)` and then immediately checks `FirstRunFlowOutcomes.IsFinished(last!)`, returning `Finished` in the same iteration when it is true. If that terminal poll response carries an outstanding machine-action request and `ReportMachineActionAsync` fails because of a non-2xx response or transport error, the action is added to `performed` but not `reported`; the immediate return prevents the intended retry, permanently losing the performed action’s outcome from the server’s perspective.

## Issue Context

Failed reports are intended to be retried on a subsequent poll tick, as documented by `FirstRunActionReportOutcome` and exercised by the existing retry test. That mechanism currently works only when polling continues: actions are marked as performed before reporting, reports are marked complete only after a successful response, and the existing test covers retries across non-terminal poll responses rather than a response that also satisfies `IsFinished`.

Preserve the perform-once guarantee while allowing an unrecorded report to retry within the polling budget before returning a finished result, and add coverage for a report failure on the terminal response.

## Fix Focus Areas

- src/Capacitor.Cli.Core/FirstRun/BrowserFirstRunFlow.cs[167-172]
- src/Capacitor.Cli.Core/FirstRun/BrowserFirstRunFlow.cs[245-258]
- test/Capacitor.Cli.Core.Tests.Unit/FirstRun/BrowserFirstRunFlowTests.cs[721-780]

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

---

Context sources

Review mode: 🧠 Deep: This is a bug-dense, cross-cutting change spanning polling, machine-action execution, CLI command behavior, wire contracts, retries, cancellation, and progress reporting, with many independent paths where redundant review is materially valuable.

Tip of the day

💡 Did you know, you can start a comment with 'qodo' or '@qodo' to chat about any finding

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗