Skip to content

Add a Profiles tab to desktop Settings - #1124

Merged
alexeyzimarev merged 36 commits into
mainfrom
capacitor/agent-c48630062fe142
Sep 23, 2026
Merged

alexeyzimarev merged 36 commits into
mainfrom
capacitor/agent-c48630062fe142

Conversation

@alexeyzimarev

Copy link
Copy Markdown
Member

Part of #1093 — AI-3072 (the first of three PRs; the last closes the issue)

What & why

The desktop app's Settings window gains a Profiles tab that lists every profile in config.json with its credential status and offers Sign in and Remove per row. Removal is one operation in Core shared with kcap profile remove: it decides on the locked config, refuses the active profile instead of resetting the selection to an empty default, and deletes the credential. Every write to a token file takes the profile's cross-process lock with a guard evaluated under it; a refresh re-reads under the lock rather than reviving a file deleted while it waited; the legacy tokens.json moves into its owner's slot before any writer of active_profile changes the selection. A sign-in commit checks the profile still names its server inside a strict config mutation, and Committed reports whether the credential landed.

Where to look

TokenStore: one lock-held write primitive and the guards under it — the config lock is never held while a token lock is taken. CommitBoundary: the precondition runs inside MutateStrictAsync, before the stamp.

Verification

  • Core 3518 tests, 0 failed (9 skipped); App 2654, 0 failed; CLI 4547 with one known hook-budget race, its class green alone (66/66)
  • dotnet build Capacitor.slnx: 0 warnings; dotnet publish -c Release grep IL[23][01][0-9]{2}: no output

🤖 Generated with Claude Code

alexeyzimarev and others added 28 commits September 21, 2026 18:12
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A switch that fails after activation is resumed by the next start; sign-in commits check the profile is still absent or still on the same server at commit time.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The CLI writes active_profile after the old owner is confirmed gone and before the new unit exists, so any later failure leaves no daemon and the next start's ordinary install converges; the app keeps no journal.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Activation rechecks the target on the locked config and re-probes both labels for a live owner; the lane refuses every request after a switch at admission; token writes go through one locked seam that spares another profile's legacy credential.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…#1093)

A runtime start can race the activation interval past every service lock, so the invariant is narrowed to daemons running at the write and the attach path compares the daemon's identity to the resolution. Logout, legacy migration and the save guard join the token lock.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Migration retires a redundant legacy file instead of skipping it, runs before the transaction's forward budget, and the daemon reports its profile so attach can tell two profiles sharing a server and name apart.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…1093)

TryLoadPure returns a fresh default alongside its false, so a guard or owner read that fails must refuse rather than decide on the default; the Activate timeout adds the token lock's wait to the rename's, and the manual repair paths name the daemon and unset the override.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
MutateAsync publishes the callback's result over a malformed config it read as defaults, so a commit-time precondition or activation recheck evaluated there passes for the wrong reason and then wipes the file.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
A save for another profile no longer deletes the active profile's legacy tokens.json.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
)

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The pre-authentication read decides LoginTarget; a profile removed or repointed while the browser was open is refused inside the strict mutation, and the token save is guarded under the profile lock.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…1093)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The precondition is an optional null-defaulted parameter at every hop from
ReauthComposition.Build to LoginAsync, and LoginTarget.SaveGuard is built from
three more, so a dropped one refuses nothing that any suite observes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`kcap use` migrates the legacy credential before it refuses, so "nothing was
changed" was false once that file had moved; a profile name the token layout
rejects threw out of the handler that tried to name its file.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign-in dereferenced the row's server url behind no check, and the settings
window's profile view model was built outside the try whose catch reports a
construction failure.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 23, 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-09-23T08:15:22.778998Z a5e1601 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

Add race-safe profile management to desktop Settings

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

Grey Divider

AI Description

• Adds a Profiles tab with credential status, guarded sign-in, and removal actions.
• Shares race-safe profile removal and strict configuration mutation across desktop and CLI.
• Serializes credential writes and safely migrates legacy tokens before selection changes.
Diagram

sequenceDiagram
    actor User
    participant View as Profiles Tab
    participant VM as Profiles VM
    participant Config as Config Mutator
    participant Gate as Auth Gate
    participant Tokens as Token Store
    participant Auth as Sign-in Flow
    User->>View: Open Settings
    View->>VM: Refresh profiles
    VM->>Config: Read config
    VM->>Gate: Grade credentials
    Gate->>Tokens: Read token
    User->>VM: Sign in or remove
    alt Sign in
        VM->>Auth: Open pinned sign-in
        Auth->>Config: Check server and stamp
        Auth->>Tokens: Guarded save
    else Remove
        VM->>Config: Strict remove
        VM->>Tokens: Guarded delete
    end
Loading
High-Level Assessment

The chosen architecture is appropriate: profile removal belongs in Core so desktop and CLI share identical invariants, while the app retains a dedicated view model for presentation. Invoking the CLI from Settings would complicate error handling and cancellation, and duplicating removal or token logic in the app would create divergent race behavior. A file watcher was also unnecessary because activation-triggered refresh provides adequate synchronization for this scope.

Files changed (48) +5025 / -197

Enhancement (13) +365 / -28
App.axaml.csCompose and refresh desktop profile settings +42/-22

Compose and refresh desktop profile settings

• Builds the Profiles view model, opens sign-in for a specific profile and server, confirms removals, and refreshes rows when Settings activates. Bound-profile app state is refreshed only when relevant.

src/Capacitor.App/App.axaml.cs

ILifecycleSurface.csDefine the profile-removal prompt kind +1/-0

Define the profile-removal prompt kind

• Adds a lifecycle prompt identifier for profile removal confirmation.

src/Capacitor.App/Services/ILifecycleSurface.cs

LifecyclePromptViewModel.csPresent profile-removal confirmation text +7/-5

Present profile-removal confirmation text

• Maps profile removal prompts to a dedicated title and Remove action label.

src/Capacitor.App/ViewModels/LifecyclePromptViewModel.cs

ProfileCredentialStatus.csDefine desktop credential status states +3/-0

Define desktop credential status states

• Introduces the credential states displayed for profile rows, including expired, mismatched, and unreadable credentials.

src/Capacitor.App/ViewModels/ProfileCredentialStatus.cs

ProfileRow.csModel profile rows and available actions +23/-0

Model profile rows and available actions

• Represents profile identity, server, active and bound markers, credential labels, and sign-in or removal eligibility.

src/Capacitor.App/ViewModels/ProfileRow.cs

ProfilesSettingsViewModel.csImplement profile listing, sign-in, and removal +153/-0

Implement profile listing, sign-in, and removal

• Reads profiles, evaluates credentials without refreshing, rejects stale rows, and coordinates guarded sign-in and shared removal. It preserves existing rows when configuration becomes unreadable.

src/Capacitor.App/ViewModels/ProfilesSettingsViewModel.cs

SettingsViewModel.csExpose profile settings to the window +4/-1

Expose profile settings to the window

• Adds an optional Profiles Settings view model to the existing Settings composition.

src/Capacitor.App/ViewModels/SettingsViewModel.cs

ProfilesSettingsView.axamlDesign the Profiles Settings tab +57/-0

Design the Profiles Settings tab

• Renders profile cards with server and credential status, active and bound markers, Sign in and Remove actions, and operation messages.

src/Capacitor.App/Views/ProfilesSettingsView.axaml

ProfilesSettingsView.axaml.csInitialize the Profiles Settings view +7/-0

Initialize the Profiles Settings view

• Adds the Avalonia code-behind required to initialize the new user control.

src/Capacitor.App/Views/ProfilesSettingsView.axaml.cs

SettingsWindow.axamlAdd Profiles to the Settings tabs +6/-0

Add Profiles to the Settings tabs

• Hosts the profile management view between Daemon and Notifications when a Profiles view model is available.

src/Capacitor.App/Views/SettingsWindow.axaml

ProfileRemoval.csCentralize race-safe profile removal +54/-0

Centralize race-safe profile removal

• Removes a non-active, non-default profile and its bindings under the config lock, then conditionally deletes its credential under the token lock.

src/Capacitor.Cli.Core/Config/ProfileRemoval.cs

ProfileRemovalOutcome.csDefine profile-removal outcomes +3/-0

Define profile-removal outcomes

• Enumerates successful, partial, refused, missing, and unreadable-config removal results.

src/Capacitor.Cli.Core/Config/ProfileRemovalOutcome.cs

ProfileRemovalResult.csReturn profile-removal details +5/-0

Return profile-removal details

• Pairs removal outcomes with optional details when a credential file must be retained.

src/Capacitor.Cli.Core/Config/ProfileRemovalResult.cs

Bug fix (16) +369 / -153
ReauthComposition.csAccept guarded reauthentication preconditions +4/-2

Accept guarded reauthentication preconditions

• Threads an optional commit precondition into the reauthentication operation graph.

src/Capacitor.App/Services/Onboarding/ReauthComposition.cs

WizardAuthBridges.csForward sign-in commit preconditions +2/-2

Forward sign-in commit preconditions

• Passes an optional profile precondition from the wizard bridge into known-server login.

src/Capacitor.App/Services/Onboarding/WizardAuthBridges.cs

WizardComposition.csCarry commit guards through wizard composition +6/-4

Carry commit guards through wizard composition

• Adds the optional commit precondition to wizard specifications and forwards it to the sign-in operation.

src/Capacitor.App/Services/Onboarding/WizardComposition.cs

SignInStepViewModel.csReject incomplete credential commits in the UI +5/-0

Reject incomplete credential commits in the UI

• Keeps sign-in unsatisfied and displays an error when authentication completed but the credential was not saved.

src/Capacitor.App/ViewModels/Onboarding/SignInStepViewModel.cs

AuthResult.csReport whether authentication saved credentials +4/-1

Report whether authentication saved credentials

• Extends committed authentication results with a CredentialSaved flag so callers can distinguish partial commits.

src/Capacitor.Cli.Core/Auth/AuthResult.cs

CommitPrecondition.csGuard sign-in commits against profile changes +20/-0

Guard sign-in commits against profile changes

• Introduces commit-time preconditions and an ExpectServer check that refuses removed or repointed profiles.

src/Capacitor.Cli.Core/Auth/CommitPrecondition.cs

CommitPreconditionFailedException.csRepresent rejected authentication commit guards +3/-0

Represent rejected authentication commit guards

• Adds the exception used to abort strict mutations when a sign-in precondition no longer holds.

src/Capacitor.Cli.Core/Auth/CommitPreconditionFailedException.cs

GuardedWriteOutcome.csClassify guarded credential writes +5/-0

Classify guarded credential writes

• Defines outcomes for successful writes, rejected guards, and unreadable configuration.

src/Capacitor.Cli.Core/Auth/GuardedWriteOutcome.cs

OnboardingFacade.csMake authentication commits race-safe +78/-18

Make authentication commits race-safe

• Checks profile preconditions inside strict config mutations, guards token saves under profile locks, and reports partial commits. Discovery now settles legacy credentials before changing the active profile.

src/Capacitor.Cli.Core/Auth/OnboardingFacade.cs

TokenStore.csSerialize all credential mutations per profile +138/-64

Serialize all credential mutations per profile

• Routes saves, deletes, refreshes, logout, and legacy migration through profile locks with optional config guards. Refresh now re-reads under the lock, preventing deleted credentials from being resurrected.

src/Capacitor.Cli.Core/Auth/TokenStore.cs

WorkOSDiscovery.csGuard WorkOS credential publication +11/-3

Guard WorkOS credential publication

• Migrates the outgoing legacy credential before profile selection changes and saves discovered credentials only while their profile still exists.

src/Capacitor.Cli.Core/Auth/WorkOSDiscovery.cs

ConfigMutator.csAdd strict locked configuration mutation +22/-4

Add strict locked configuration mutation

• Adds mutation APIs that refuse unreadable configuration instead of applying changes to a synthesized default.

src/Capacitor.Cli.Core/Config/ConfigMutator.cs

ConfigUnreadableException.csRepresent unsafe configuration reads +9/-0

Represent unsafe configuration reads

• Defines the failure raised when a strict mutation encounters an existing but unreadable configuration file.

src/Capacitor.Cli.Core/Config/ConfigUnreadableException.cs

LoginCommand.csFail login when credentials are not saved +1/-1

Fail login when credentials are not saved

• Returns a nonzero exit code for committed authentication whose credential publication failed or was refused.

src/Capacitor.Cli/Commands/LoginCommand.cs

ProfileCommand.csUse shared profile removal from the CLI +24/-32

Use shared profile removal from the CLI

• Delegates removal to Core, deletes credentials, refuses active profiles, and reports partial deletion or unreadable configuration clearly.

src/Capacitor.Cli/Commands/ProfileCommand.cs

UseCommand.csMake profile selection durable and guarded +37/-22

Make profile selection durable and guarded

• Migrates the outgoing legacy credential before global selection and validates the target inside a strict config mutation. Missing profiles or unreadable configuration leave selection unchanged.

src/Capacitor.Cli/Commands/UseCommand.cs

Tests (14) +985 / -15
ProfilesSettingsViewModelTests.csTest profile rows and desktop actions +162/-0

Test profile rows and desktop actions

• Covers status mapping, active and bound markers, stale-row rejection, sign-in routing, removal confirmation, token deletion, and unreadable configuration.

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

ReauthCompositionTests.csTest reauthentication precondition propagation +31/-0

Test reauthentication precondition propagation

• Verifies that the expected-server guard reaches the composed authentication facade specification.

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

SettingsWindowSmokeTests.csSmoke-test the Profiles tab +55/-4

Smoke-test the Profiles tab

• Verifies profile rows, action chips, status text, active and bound marks, and correct tab indexing for existing notification tests.

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

SignInStepViewModelTests.csTest unsaved credential presentation +20/-0

Test unsaved credential presentation

• Ensures a partial authentication commit remains unsatisfied and displays an error.

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

WizardCompositionTests.csTest guarded wizard sign-in +28/-0

Test guarded wizard sign-in

• Confirms the Paste sign-in path refuses credentials when the profile no longer names the expected server.

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

CommitBoundaryTests.csTest guarded commit and legacy settlement races +52/-0

Test guarded commit and legacy settlement races

• Ensures a delayed token save cannot revive a removed profile and discovery preserves the outgoing profile's legacy credential.

test/Capacitor.Cli.Core.Tests.Unit/Auth/CommitBoundaryTests.cs

CrossProcessRefreshTests.csTest lock-held credential refresh behavior +36/-0

Test lock-held credential refresh behavior

• Verifies refresh does not resurrect a credential deleted while waiting and still migrates legacy-only credentials safely.

test/Capacitor.Cli.Core.Tests.Unit/Auth/CrossProcessRefreshTests.cs

LoginTargetSaveGuardTests.csTest login target save-guard rules +61/-0

Test login target save-guard rules

• Covers foreign, adopting, absent, existing, and expected-server profile conditions for credential publication.

test/Capacitor.Cli.Core.Tests.Unit/Auth/LoginTargetSaveGuardTests.cs

OnboardingFacadeTests.csTest commit-time profile preconditions +84/-0

Test commit-time profile preconditions

• Covers successful guarded commits and refusal after profile removal, repointing, or configuration corruption while preserving foreign-login behavior.

test/Capacitor.Cli.Core.Tests.Unit/Auth/OnboardingFacadeTests.cs

TokenStoreProfileTests.csTest serialized token mutation and migration +198/-3

Test serialized token mutation and migration

• Adds coverage for guarded saves and deletes, lock contention, owner-aware legacy handling, locked logout, migration failures, and per-profile isolation.

test/Capacitor.Cli.Core.Tests.Unit/Auth/TokenStoreProfileTests.cs

ConfigMutatorTests.csTest strict configuration mutation +22/-0

Test strict configuration mutation

• Verifies malformed configuration is preserved without invoking the mutation and absent configuration can still be created.

test/Capacitor.Cli.Core.Tests.Unit/Config/ConfigMutatorTests.cs

ProfileRemovalTests.csTest shared profile removal outcomes +123/-0

Test shared profile removal outcomes

• Covers profile and binding deletion, active/default/missing refusals, unreadable config, case aliases, invalid names, and retained-token reporting.

test/Capacitor.Cli.Core.Tests.Unit/Config/ProfileRemovalTests.cs

ProfileCommandTests.csTest CLI profile removal semantics +43/-4

Test CLI profile removal semantics

• Updates construction for TokenStore injection and verifies credential deletion, active-profile refusal, and unknown-profile failure.

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

UseCommandTests.csTest guarded profile selection +70/-4

Test guarded profile selection

• Covers strict target validation, legacy credential migration for global selection, migration failure, and unreadable configuration handling.

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

Documentation (5) +3306 / -1
README.mdDocument destructive profile removal behavior +2/-0

Document destructive profile removal behavior

• Explains that profile removal also deletes its saved credential and refuses the active profile.

README.md

CHANGES.mdRecord profile management and credential-locking changes +12/-0

Record profile management and credential-locking changes

• Documents the Profiles tab, shared removal operation, strict config mutations, guarded token writes, and legacy migration behavior.

docs/CHANGES.md

2026-09-22-ai3072-profiles-settings-pr1.mdAdd the Profiles Settings implementation plan +2492/-0

Add the Profiles Settings implementation plan

• Provides the task-by-task implementation, testing, delivery, and verification plan for the first Profiles Settings PR.

docs/superpowers/plans/2026-09-22-ai3072-profiles-settings-pr1.md

2026-09-21-ai3072-desktop-profiles-settings-design.mdSpecify desktop profile management architecture +799/-0

Specify desktop profile management architecture

• Defines the complete multi-PR design for listing, signing in, adding, removing, and switching profiles, including concurrency and daemon invariants.

docs/superpowers/specs/2026-09-21-ai3072-desktop-profiles-settings-design.md

help-profile.txtClarify profile removal help +1/-1

Clarify profile removal help

• States that removal deletes saved sign-in data and cannot target active or default profiles.

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

@qodo-code-review

qodo-code-review Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Action required

1. Logout can leave legacy sign-ins behind ✓ Resolved 🐞 Bug ⛨ Security
Description
TokenStore.DeleteAsync passes cfg.ActiveName to AcquireProfileLockAsync and silently catches
the resulting ArgumentException when that configured name cannot form a token filename. Profiles
such as bad/name are accepted by configuration and profile creation, so logout skips tokens.json
and the CLI nevertheless prints that logout succeeded.
Code

src/Capacitor.Cli.Core/Auth/TokenStore.cs[R324-327]

+        try {
+            using var lockStream = await AcquireProfileLockAsync(cfg.ActiveName, ct);
+            if (File.Exists(LegacyTokenPath)) File.Delete(LegacyTokenPath);
+        } catch (Exception ex) when (ex is not OperationCanceledException) { /* best-effort */ }
Relevance

●●● Strong

Accepted token-storage precedents require validating profile-derived paths and handling invalid
profile names safely.

PR-#171
PR-#556

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Token locks reject path separators, but profile creation persists names without equivalent
validation and the repository explicitly tests bad/name as accepted configuration. The new logout
block suppresses that validation exception, after which the top-level logout command unconditionally
reports success.

src/Capacitor.Cli.Core/Auth/TokenStore.cs[64-74]
src/Capacitor.Cli.Core/Auth/TokenStore.cs[324-327]
src/Capacitor.Cli/Commands/ProfileCommand.cs[53-75]
test/Capacitor.Cli.Core.Tests.Unit/Config/ProfileRemovalTests.cs[95-109]
src/Capacitor.Cli/Program.cs[375-380]

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

## Issue description
Logout silently retains the legacy credential when the active configuration profile has a name rejected by the token filename validator.

## Fix Focus Areas
- src/Capacitor.Cli.Core/Auth/TokenStore.cs[64-74]
- src/Capacitor.Cli.Core/Auth/TokenStore.cs[309-327]
- src/Capacitor.Cli/Commands/ProfileCommand.cs[53-75]

## Recommended Fix
Make lock-file derivation safe for every profile name accepted by configuration, for example by encoding or hashing lock names independently from token filenames. Ensure logout can lock and delete the legacy credential for existing malformed names, and add a regression test asserting `tokens.json` is gone before logout reports success.

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


2. Failed global switches still move credentials ✓ Resolved 🐞 Bug ≡ Correctness
Description
UseCommand.SetProfile calls MigrateLegacyAsync(before.ActiveName) before its strict mutation
verifies that name exists. A global kcap use request for an unknown profile can therefore move
or delete the legacy credential and then return the unknown-profile error without changing
active_profile.
Code

src/Capacitor.Cli/Commands/UseCommand.cs[R33-40]

+        if (selectsGlobally) {
+            if (!ConfigMutator.TryLoadPure(AppConfig.GetConfigPath(config), out var before)) {
+                await Console.Error.WriteLineAsync("The configuration file could not be read; nothing was changed.");
+                return 1;
+            }
+            try {
+                await tokens.MigrateLegacyAsync(before.ActiveName);
+            } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or TimeoutException) {
Relevance

●●● Strong

Accepted migration precedents emphasize resolving and stabilizing profile state before token
movement or mutation.

PR-#257
PR-#387

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new migration is ordered ahead of the newly added strict target lookup, while migration moves
the legacy file or deletes it if an owner file exists.

src/Capacitor.Cli/Commands/UseCommand.cs[32-58]
src/Capacitor.Cli.Core/Auth/TokenStore.cs[337-349]
test/Capacitor.Cli.Tests.Unit/Commands/UseCommandTests.cs[113-121]

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

## Issue description
`kcap use <unknown> --global` migrates the outgoing legacy credential before checking whether the requested profile exists, so a command that fails can still change credential storage.

## Fix Focus Areas
- src/Capacitor.Cli/Commands/UseCommand.cs[33-53]

## Recommended Fix
Validate that the requested global target exists from the same fresh configuration snapshot before calling `MigrateLegacyAsync`. Preserve a strict locked validation before publishing `active_profile` so a concurrent removal is still rejected, but do not migrate credentials when the requested target is already known to be absent.

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


3. Global profile switches can crash ✓ Resolved 🐞 Bug ☼ Reliability
Description
UseCommand.SetProfile calls MigrateLegacyAsync, but its exception filter omits the
ArgumentException raised when TokenStore validates configured profile names that are invalid
filename components, even though ProfileCommand.AddProfile accepts and stores names such as
bad/name. If such an active profile owns an existing legacy credential, any global switch fails
before changing the selection or returning a controlled command error, and discovery reaches the
same incomplete migration exception handling.
Code

src/Capacitor.Cli/Commands/UseCommand.cs[R38-40]

+            try {
+                await tokens.MigrateLegacyAsync(before.ActiveName);
+            } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or TimeoutException) {
Relevance

●●● Strong

Accepted precedents require defensive handling around profile validation and migration failures
instead of allowing reachable exceptions to escape.

PR-#171
PR-#640

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The profile-add path persists the supplied name without applying the stricter token filename rules,
while MigrateLegacyAsync validates the active profile name when acquiring its lock path through
TokenStore. Both newly added migration callers catch only I/O, authorization, and timeout
failures, so a reachable ArgumentException from filename validation remains unhandled.

src/Capacitor.Cli/Commands/UseCommand.cs[32-43]
src/Capacitor.Cli.Core/Auth/OnboardingFacade.cs[114-122]
src/Capacitor.Cli.Core/Auth/TokenStore.cs[64-74]
src/Capacitor.Cli.Core/Auth/TokenStore.cs[337-348]
src/Capacitor.Cli/Commands/ProfileCommand.cs[53-75]
src/Capacitor.Cli/Commands/UseCommand.cs[32-44]
src/Capacitor.Cli/Commands/ProfileCommand.cs[48-67]
src/Capacitor.Cli.Core/Auth/TokenStore.cs[67-89]

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

## Issue description
Legacy migration can throw an unhandled `ArgumentException` when an active configuration profile has a name accepted by `profile add` but rejected by the token filename layout, such as a name containing a path separator. With an existing legacy token, this affects global profile switching and discovery instead of producing a controlled, actionable CLI error.

## Fix Focus Areas
- src/Capacitor.Cli/Commands/UseCommand.cs[32-44]
- src/Capacitor.Cli.Core/Auth/OnboardingFacade.cs[114-122]
- src/Capacitor.Cli.Core/Auth/TokenStore.cs[337-349]

## Recommended Fix
Support legacy migration for all accepted profile names, or catch profile-name validation failures from `MigrateLegacyAsync` at every migration caller alongside the existing migration failures and route them through the same nonzero, actionable CLI error path. If every profile must be token-file compatible, also validate names when profiles are created, while retaining caller-side handling for existing configuration files; add tests for global switching and discovery when an invalid active profile name owns an existing legacy token.

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


View high (2)
4. Removed profiles retain credentials on Unix ✓ Resolved 🐞 Bug ≡ Correctness
Description
ProfileRemoval.RemoveAsync suppresses deletion whenever any remaining profile name equals the
removed name ignoring case. On a case-sensitive filesystem, acme.json and Acme.json are separate
exact-name token paths, so removing acme while Acme remains leaves the removed profile's
credential despite reporting successful removal.
Code

src/Capacitor.Cli.Core/Config/ProfileRemoval.cs[R36-37]

+            var outcome = await tokens.DeleteGuardedAsync(name,
+                cfg => !cfg.Profiles.Keys.Any(k => string.Equals(k, name, StringComparison.OrdinalIgnoreCase)), ct);
Relevance

●● Moderate

The platform-specific case-collision issue is credible, but no closely matching profile-removal
precedent appeared.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Removal deletes only the exact config key but uses a case-insensitive credential guard; TokenStore
builds credential paths from the exact supplied profile string, and the added test currently asserts
retention without platform distinction.

src/Capacitor.Cli.Core/Config/ProfileRemoval.cs[15-25]
src/Capacitor.Cli.Core/Config/ProfileRemoval.cs[33-41]
src/Capacitor.Cli.Core/Auth/TokenStore.cs[84-89]
test/Capacitor.Cli.Core.Tests.Unit/Config/ProfileRemovalTests.cs[82-95]

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

## Issue description
Profile removal treats case-only profile aliases as sharing a credential file on every platform. On case-sensitive filesystems the token path uses the exact profile name, so this retains an orphaned credential for the removed profile.

## Fix Focus Areas
- src/Capacitor.Cli.Core/Config/ProfileRemoval.cs[33-41]

## Recommended Fix
Make the delete guard protect only profiles that resolve to the same token path on the current filesystem. On case-sensitive platforms, check for an exact remaining profile name so removing `acme` deletes `acme.json` even when `Acme` remains; retain case-insensitive alias protection only where those names share a file.

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


5. Logout can preserve a refreshed sign-in ✓ Resolved 🐞 Bug ⛨ Security
Description
TokenStore.DeleteAsync releases every profile lock before acquiring the active profile lock to
remove the legacy credential. When a legacy-only refresh runs in that gap, it can recreate the
active profile file before logout removes only tokens.json, so kcap logout returns with a usable
credential still stored.
Code

src/Capacitor.Cli.Core/Auth/TokenStore.cs[R324-326]

+        try {
+            using var lockStream = await AcquireProfileLockAsync(cfg.ActiveName, ct);
+            if (File.Exists(LegacyTokenPath)) File.Delete(LegacyTokenPath);
Relevance

●● Moderate

The race is security-relevant and resembles accepted token-lock ordering fixes, but no direct
logout-refresh precedent appeared.

PR-#257

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Logout completes its per-profile deletion loop before taking the active profile lock, while refresh
falls back to the legacy file and saves the refreshed credential under that same lock. This ordering
permits refresh to recreate the profile file after its logout deletion but before the legacy
deletion.

src/Capacitor.Cli.Core/Auth/TokenStore.cs[319-327]
src/Capacitor.Cli.Core/Auth/TokenStore.cs[643-656]
src/Capacitor.Cli.Core/Auth/TokenStore.cs[675-678]

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

## Issue description
Logout deletes the active profile file and legacy token in separate critical sections, allowing refresh to recreate the profile credential between them.

## Fix Focus Areas
- src/Capacitor.Cli.Core/Auth/TokenStore.cs[306-327]
- src/Capacitor.Cli.Core/Auth/TokenStore.cs[643-678]

## Recommended Fix
Handle the active profile specially: acquire its profile lock once and, while retaining it, delete both its profile token file and the legacy token file. Delete other profile files under their respective locks and add a concurrency test where a legacy refresh overlaps logout.

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



Remediation recommended

6. Older refreshes can replace newer rows ✓ Resolved 🐞 Bug ☼ Reliability
Description
OpenSettings and the initial load start untracked ProfilesSettingsViewModel.RefreshAsync calls
without cancellation, serialization, or a generation check before assigning Rows. Because each
invocation snapshots configuration before awaiting per-profile credential or token-status I/O, an
older slower refresh can complete after a newer one and restore outdated profiles or credential
statuses, including profiles an external command changed or removed.
Code

src/Capacitor.App/App.axaml.cs[R777-780]

+        // The list is re-read whenever the window regains focus, so a `kcap use` or `kcap profile
+        // add` made in a terminal shows up on return without a file watcher.
+        window.Activated += (_, _) => _ = profilesVm.RefreshAsync();
+        _ = profilesVm.RefreshAsync();
Relevance

●●● Strong

Recent accepted precedents explicitly require generation or serialization for overlapping
asynchronous refresh publications.

PR-#1069
PR-#831

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Both startup and activation invoke independent fire-and-forget refresh tasks, while each
RefreshAsync call reads a configuration snapshot, awaits per-profile status evaluation, and then
publishes Rows unconditionally. With no mutual exclusion, cancellation, or generation check before
that final assignment, task completion order determines which snapshot remains visible.

src/Capacitor.App/App.axaml.cs[777-780]
src/Capacitor.App/ViewModels/ProfilesSettingsViewModel.cs[60-72]
src/Capacitor.App/ViewModels/ProfilesSettingsViewModel.cs[82-95]
src/Capacitor.App/ViewModels/ProfilesSettingsViewModel.cs[59-71]
src/Capacitor.App/ViewModels/ProfilesSettingsViewModel.cs[75-95]
src/Capacitor.App/Services/Onboarding/OnboardingGate.cs[51-87]

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

## Issue description
Multiple fire-and-forget settings refreshes can run concurrently and complete out of order. An earlier refresh delayed by credential or token-status evaluation can publish an older configuration snapshot after a later refresh has already displayed current profiles and statuses.

## Fix Focus Areas
- src/Capacitor.App/App.axaml.cs[777-780]
- src/Capacitor.App/ViewModels/ProfilesSettingsViewModel.cs[60-72]

## Recommended Fix
Move refresh coordination into `ProfilesSettingsViewModel` and route startup and activation-triggered refreshes through the coordinated method. Serialize refreshes, cancel superseded requests, or use a monotonically increasing generation so that `Rows` is published only when the completing invocation is still current; also track fire-and-forget failures and add an out-of-order completion test where configuration changes between two refreshes.

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


7. Discovery reports an unsaved login ✓ Resolved 📘 Rule violation ≡ Correctness
Description
CommitBoundary.CommitAsync can now return AuthResult.Committed with CredentialSaved: false,
but LoginCommand.MapDiscoverResult and setup's discovery handling classify every Committed value
as success. This occurs when the guarded WorkOS token write loses a race with profile removal,
causing discovery to return exit code 0 and setup to continue without the credential.
Code

src/Capacitor.Cli.Core/Auth/OnboardingFacade.cs[R92-93]

+            request.ActiveProfile, request.CanonicalServer, request.Provider, username, request.Identities,
+            CredentialSaved: request.PublishTokens is null || tokenSaved);
Relevance

●●● Strong

Accepted auth precedents require distinct handling when authentication completes without producing a
usable credential.

PR-#387
PR-#640

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 2750487 requires every auth result outcome to be handled distinctly without silently accepting
an incomplete commit. The changed commit boundary creates a committed result with no saved
credential, the guarded WorkOS write can produce that state, and existing discovery/setup branches
accept it as success.

Rule 2750487: Enforce ordered and exhaustive auth commit handling
src/Capacitor.Cli.Core/Auth/OnboardingFacade.cs[89-93]
src/Capacitor.Cli.Core/Auth/WorkOSDiscovery.cs[234-244]
src/Capacitor.Cli/Commands/LoginCommand.cs[55-68]
src/Capacitor.Cli/Commands/SetupCommand.cs[2088-2125]

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

## Issue description
The new `CredentialSaved: false` outcome represents a configuration commit whose credential was not saved, but discovery and setup still treat every `AuthResult.Committed` as successful.

## Fix Focus Areas
- src/Capacitor.Cli/Commands/LoginCommand.cs[55-68]
- src/Capacitor.Cli/Commands/SetupCommand.cs[2088-2125]
- src/Capacitor.Cli.Core/Auth/OnboardingFacade.cs[89-93]

## Recommended Fix
Add explicit `AuthResult.Committed { CredentialSaved: false }` branches before general committed branches. Return failure from CLI discovery and prevent setup from continuing as authenticated, while preserving the distinct partial-commit result for recovery and reporting.

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


8. Action guard comment misstates failure ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
ProfilesSettingsViewModel.CurrentAsync treats retained Rows as fresh even though RefreshAsync
returns after an unreadable configuration without clearing them or communicating failure,
contradicting the guard comment that every action rejects rows that no longer match the file. If
config.json becomes unreadable after rows were loaded, the guard can return a stale matching row,
allowing sign-in to reach _openSignIn and removal to request confirmation before later guarded
operations reject the state.
Code

src/Capacitor.App/ViewModels/ProfilesSettingsViewModel.cs[R141-143]

+    // Every action re-reads first: a row that no longer matches the file is refused, not acted on.
+    async Task<ProfileRow?> CurrentAsync(ProfileRow row) {
+        await RefreshAsync();
Relevance

●● Moderate

The stale-row failure is plausible, but history provides no closely matching accepted or rejected
precedent for this exact guard behavior.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 2762993 requires changed comments to remain true and document current behavior, but the
action-guard comment says every action rejects stale rows while RefreshAsync sets a read-error
message and intentionally retains the prior rows on failure. Because CurrentAsync does not inspect
that failure and can select a retained row whose name and server match, while strict mutations
elsewhere treat unreadable configuration as a failure state, the cited paths show that the comment
and validation behavior are inconsistent.

Rule 2762993: Restrict comments to documenting non-obvious, behavior‑critical constraints
src/Capacitor.App/ViewModels/ProfilesSettingsViewModel.cs[59-64]
src/Capacitor.App/ViewModels/ProfilesSettingsViewModel.cs[141-148]
src/Capacitor.App/ViewModels/ProfilesSettingsViewModel.cs[98-107]
src/Capacitor.App/ViewModels/ProfilesSettingsViewModel.cs[98-125]
src/Capacitor.App/ViewModels/ProfilesSettingsViewModel.cs[141-151]
src/Capacitor.Cli.Core/Config/ConfigMutator.cs[36-43]

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

## Issue description
Action validation cannot distinguish a successful refresh from one that retained stale rows after a configuration read failure. As a result, `CurrentAsync` can return an old row despite the comment promising that stale rows are rejected.

## Fix Focus Areas
- src/Capacitor.App/ViewModels/ProfilesSettingsViewModel.cs[59-72]
- src/Capacitor.App/ViewModels/ProfilesSettingsViewModel.cs[98-125]
- src/Capacitor.App/ViewModels/ProfilesSettingsViewModel.cs[141-151]

## Recommended Fix
Make `RefreshAsync` communicate whether the configuration read succeeded, invalidate `Rows` on failure, or record a current snapshot generation. Update `CurrentAsync` to return `null` immediately whenever the fresh configuration cannot be read, while preserving the read-error message and preventing sign-in or removal confirmation from starting against retained rows. Keep the action-guard comment only once the implementation rejects actions for both mismatched and unreadable state.

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


Grey Divider

Context sources
✅ Compliance rules (platform): 64 rules
✅ Cross-repo context — repo relationships
  Explored: repo: kurrent-io/kcap-deployments (sha: 02770a19)
  Explored: repo: kurrent-io/kcap-server (sha: f3067f6f)
Review mode: 🧠 Deep: This is a high-density, cross-cutting change spanning desktop UI, shared authentication/token locking, config mutation, CLI behavior, and concurrency-sensitive flows, with 98 edit sites and many plausible independent defects.

Grey Divider

Tip of the day
💡 Did you know, you can keep summaries lean with Finding overflow, which tucks the rest behind 'View more'

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/Capacitor.Cli.Core/Auth/OnboardingFacade.cs
Comment thread src/Capacitor.App/ViewModels/ProfilesSettingsViewModel.cs Outdated
Comment thread src/Capacitor.Cli.Core/Auth/TokenStore.cs
Comment thread src/Capacitor.Cli.Core/Auth/TokenStore.cs
Comment thread src/Capacitor.Cli/Commands/UseCommand.cs Outdated
Comment thread src/Capacitor.App/App.axaml.cs
Comment thread src/Capacitor.Cli/Commands/UseCommand.cs Outdated
Comment thread src/Capacitor.Cli.Core/Config/ProfileRemoval.cs Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a5e1601332

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

_time,
WizardComposition.NewOperation);
WizardComposition.NewOperation,
new CommitPrecondition.ExpectServer(serverUrl));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Limit the server precondition to profile-row sign-ins

When the desktop app is launched with KCAP_URL, ProfileResolver intentionally resolves a server without a profile, so the Home sign-in path passes the fallback active profile name together with the override URL. This unconditional ExpectServer then rejects the commit unless that on-disk profile already names the override URL, meaning users complete authentication but cannot save credentials for the server the app is actually using. Pass this precondition only for Settings row sign-ins, or make it nullable for the existing Home re-auth flow.

Useful? React with 👍 / 👎.

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.

Deliberate, so keeping it. Under KCAP_URL the resolution has no profile, and before this change the rail's sign-in adopted the override into the on-disk profile — server_url of work silently rewritten to whatever the shell exported, and the credential stamped for it. The precondition refuses that with profile 'work' does not name <url>; nothing saved. Signing in against an override is done with a profile that names that server (kcap setup <url>, or KCAP_PROFILE instead of KCAP_URL); the design leaves the URL-launch case out of the Profiles work.

alexeyzimarev and others added 8 commits September 23, 2026 10:35
The Settings tab order is Daemon, Profiles, Notifications, so main's new
notification-access smoke test selects the Notifications tab at index 2.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Logout deletes the active profile's file and the legacy file under one hold of
its lock, and a name the layout rejects still gets the legacy file deleted. A
case-alias shares a token file only where the directory lists one spelling.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… locks (#1093)

An action judges its row against its own fresh read, since a superseded read
never publishes and the shared rows may be a newer read's still in progress.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@alexeyzimarev
alexeyzimarev merged commit ba6a357 into main Sep 23, 2026
8 checks passed
@alexeyzimarev
alexeyzimarev deleted the capacitor/agent-c48630062fe142 branch September 23, 2026 15:31
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