Remove dead code, stale comments and low-value tests from the desktop app

[nortonandreev]

AI-3144 — no GitHub issue yet

What & why

Removes dead and unreachable code from the desktop app. Rewrites comments that are wrong about the current code or break the repo's comment rules. Prunes app tests that pin only constants, one-line forwards, or behaviour a named sibling test already covers. No UI or wire changes: the hub payload options, brushes and animation are value-identical.

Where to look

• ServerConnectionService.Build now calls LaunchHubJson.Configure, so the wire-contract tests use the same options production does. Before, they checked a copy production never called.
• RemoteSessionViewModelTests.A_same_id_local_row_for_another_session_is_not_an_origin_change now marks the agent as a proven twin. It covers a case it previously skipped.
• MarkdownImage caches its ancestors when attached, instead of walking the tree on every layout pass.

Verification

• dotnet build test/Capacitor.App.Tests.Unit: 0 warnings, 0 errors (warnings are errors).
• dotnet run --project test/Capacitor.App.Tests.Unit: 2746/2746 passed. The baseline on main was 2804/2804; the difference is the deleted tests.

[nortonandreev]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Can't wait for the next one!

Reviewed commit: 37ab27ef3b

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

[qodo-code-review]

PR Summary by Qodo

Audit and simplify desktop app code and tests

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Removes unreachable members, redundant branches, and unnecessary allocations across the desktop
 app.
• Aligns production SignalR serialization with wire-contract tests and caches Markdown image
 ancestors.
• Prunes duplicate, forwarding-only, and constant-pinning tests while correcting one remote-session
 fixture.

Diagram

graph TD
  App["App composition"] --> Services["Desktop services"] --> Models["View models"] --> Views["Avalonia views"]
  Services --> Connection["Server connection"] --> Json["Launch JSON"] --> Hub["SignalR hub"]
  Tests["Unit tests"] --> Json

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Split behavioral changes from cleanup

• ➕ Narrows review scope for SignalR, caching, and cancellation changes
• ➕ Makes test deletions independently auditable
• ➕ Reduces the risk that functional changes hide inside comment churn
• ➖ Adds coordination and CI overhead
• ➖ Shared constant moves and related test cleanup would cross PR boundaries

Recommendation: Split the small functional and structural changes from the broad comment/test audit, or at least preserve that separation in commits. The current edits are coherent as an audit, but production changes such as shared hub serialization, exception-flow simplification, ancestor caching, and interface contraction deserve a focused review distinct from low-value test removal.

Files changed (159) +778 / -2014

Enhancement (2) +11 / -9

ServerConnectionService.csApply shared launch JSON options in production +3/-6

Apply shared launch JSON options in production

• Routes SignalR payload configuration through LaunchHubJson and narrows unconditional signed-out parking to internal use.

src/Capacitor.App/Services/ServerConnectionService.cs

MarkdownImage.csCache Markdown image visual ancestors +8/-3

Cache Markdown image visual ancestors

• Finds the containing markdown pane and host once per visual-tree attachment instead of every layout pass.

src/Capacitor.App/Views/MarkdownImage.cs

Refactor (47) +260 / -399

App.axamlConsolidate shared pulse animation +8/-25

Consolidate shared pulse animation

• Reuses one pulse animation for status controls and running tool rows while shortening palette comments.

src/Capacitor.App/App.axaml

App.axaml.csRemove stale composition state and simplify app wiring +55/-99

Remove stale composition state and simplify app wiring

• Drops unused fields, reuses the resolved server HTTP client, and rewrites obsolete specification-oriented comments without changing composition behavior.

src/Capacitor.App/App.axaml.cs

BlockTidy.csSimplify block wrapper matching +1/-1

Simplify block wrapper matching

• Reorders the pattern check to bind the container once before checking supported wrapper types.

src/Capacitor.App/GitHubHtml/BlockTidy.cs

AgentDirectory.csSimplify placeholder expiry and row reconciliation +4/-4

Simplify placeholder expiry and row reconciliation

• Relies on expired placeholders already being removed and precomputes row keys for cache reconciliation.

src/Capacitor.App/Services/AgentDirectory.cs

DaemonLifecycleController.csRemove unused success exit code +13/-14

Remove unused success exit code

• Deletes the unused zero exit-code constant and refreshes lifecycle matrix documentation.

src/Capacitor.App/Services/DaemonLifecycleController.cs

HostedHarnessCatalog.csRemove redundant harness projection +2/-3

Remove redundant harness projection

• Constructs harness options directly from registry identities and removes fixed-count wording.

src/Capacitor.App/Services/HostedHarnessCatalog.cs

ILaunchClient.csRemove unused launch JSON source context +5/-15

Remove unused launch JSON source context

• Deletes the unused source-generated serializer context and corrects launch payload wire documentation.

src/Capacitor.App/Services/ILaunchClient.cs

LaunchHubJson.csReduce launch hub JSON configuration +4/-12

Reduce launch hub JSON configuration

• Removes the unused generated resolver and retains the snake_case naming policy shared with production and tests.

src/Capacitor.App/Services/LaunchHubJson.cs

DaemonMutationLane.csShare the consent capability constant +5/-7

Share the consent capability constant

• Uses ConsentFlipCoordinator's public consent/3 capability constant instead of maintaining a duplicate.

src/Capacitor.App/Services/Mutation/DaemonMutationLane.cs

OutcomeChannel.csDeduplicate outcome requeue logic +13/-13

Deduplicate outcome requeue logic

• Extracts the exactly-once requeue and second-abandonment logging behavior into one helper.

src/Capacitor.App/Services/Mutation/OutcomeChannel.cs

ConsentFlipClaims.csCentralize consent identity canonicalization +8/-3

Centralize consent identity canonicalization

• Adds a shared canonicalization helper used when matching and consuming consent-flip claims.

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

ConsentFlipCoordinator.csExpose shared consent capability and canonicalization +10/-18

Expose shared consent capability and canonicalization

• Publishes the consent/3 capability, delegates identity normalization to the claims store, and removes an unused instance acknowledgment method.

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

PermissionService.csUse an immutable empty session map +1/-1

Use an immutable empty session map

• Initializes the session-agent map with FrozenDictionary.Empty instead of allocating a mutable dictionary.

src/Capacitor.App/Services/PermissionService.cs

ServerWorkContextSource.csReuse authenticated read factory type +2/-4

Reuse authenticated read factory type

• Removes a duplicate delegate and passes the shared generic client factory directly.

src/Capacitor.App/Services/ServerWorkContextSource.cs

TerminalFeedSanitizer.csRemove unreachable empty SGR branch +0/-1

Remove unreachable empty SGR branch

• Deletes a branch that cannot be reached after the sanitizer's rewrite predicate.

src/Capacitor.App/Services/TerminalFeedSanitizer.cs

IAppUpdater.csRemove unused download progress parameter +1/-1

Remove unused download progress parameter

• Contracts updater downloads to candidate and cancellation because no caller consumes progress.

src/Capacitor.App/Services/Update/IAppUpdater.cs

InertAppUpdater.csMatch simplified updater interface +1/-1

Match simplified updater interface

• Removes the unused progress parameter from the inert updater implementation.

src/Capacitor.App/Services/Update/InertAppUpdater.cs

UpdateCoordinator.csUse simplified updater download contract +4/-4

Use simplified updater download contract

• Calls the updater without an unused progress argument and corrects synchronization documentation.

src/Capacitor.App/Services/Update/UpdateCoordinator.cs

VelopackAppUpdater.csRemove unused Velopack progress forwarding +2/-2

Remove unused Velopack progress forwarding

• Drops progress plumbing and invokes Velopack downloads with no progress callback.

src/Capacitor.App/Services/Update/VelopackAppUpdater.cs

AttachmentTray.csRemove unused attachment byte total +0/-2

Remove unused attachment byte total

• Deletes the unconsumed TotalBytes property and its property-change notification.

src/Capacitor.App/ViewModels/AttachmentTray.cs

ChatTabViewModel.csRemove unused root observable +0/-1

Remove unused root observable

• Deletes the unconsumed public projection of the internal root subject.

src/Capacitor.App/ViewModels/ChatTabViewModel.cs

HomeViewModel.csReuse attachment capability constants +8/-12

Reuse attachment capability constants

• Moves local attachment capability and upgrade copy to LocalFrameChatInput and corrects launcher comments.

src/Capacitor.App/ViewModels/HomeViewModel.cs

LocalFrameChatInput.csOwn local attachment capability constants +4/-4

Own local attachment capability constants

• Centralizes local attachment capability text and lets the general exception handler cover cancellation.

src/Capacitor.App/ViewModels/LocalFrameChatInput.cs

AgentsStepViewModel.csRemove unused agents-step idle projection +1/-4

Remove unused agents-step idle projection

• Deletes the unconsumed Idle property and its change notification.

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

DaemonStepViewModel.csShare consent constants and identity normalization +12/-19

Share consent constants and identity normalization

• Removes duplicate capability and canonicalization logic and deletes an unused Idle projection.

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

ImportStepViewModel.csRemove unused import-step idle projection +2/-5

Remove unused import-step idle projection

• Deletes the unconsumed Idle property and refreshes import cancellation comments.

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

PlanTaskRow.csRemove unused plan-task state projections +0/-4

Remove unused plan-task state projections

• Deletes unconsumed pending and skipped flags and their property-change notifications.

src/Capacitor.App/ViewModels/PlanTaskRow.cs

PullRequestContextViewModel.Presentation.csCache presentation property names +9/-6

Cache presentation property names

• Moves the repeated notification property array to a static field to avoid per-notification allocation.

src/Capacitor.App/ViewModels/PullRequestContextViewModel.Presentation.cs

PullRequestContextViewModel.Projections.csRemove unused pull-request projections +10/-10

Remove unused pull-request projections

• Deletes unconsumed lifecycle and review-summary properties and caches the retained notification names.

src/Capacitor.App/ViewModels/PullRequestContextViewModel.Projections.cs

PullRequestContextViewModel.Reads.csStop storing unused page metadata +2/-2

Stop storing unused page metadata

• Removes assignments for section metadata that no presentation or decision consumes.

src/Capacitor.App/ViewModels/PullRequestContextViewModel.Reads.cs

PullRequestSectionState.csRemove unused pull-request section fields +0/-3

Remove unused pull-request section fields

• Deletes unconsumed started, fetched, and coverage-reason state.

src/Capacitor.App/ViewModels/PullRequestSectionState.cs

RemoteSessionViewModel.csRemove unused access-state observable +0/-1

Remove unused access-state observable

• Deletes the unconsumed public projection of the internal access state subject.

src/Capacitor.App/ViewModels/RemoteSessionViewModel.cs

RemoteTranscriptFeed.csReuse locked offset calculation +1/-1

Reuse locked offset calculation

• Delegates the public offset getter to the existing lock-safe helper.

src/Capacitor.App/ViewModels/RemoteTranscriptFeed.cs

ServerChatInput.csConsolidate chat send failure handling +0/-2

Consolidate chat send failure handling

• Lets the existing general exception path handle cancellation as an unconfirmed delivery.

src/Capacitor.App/ViewModels/ServerChatInput.cs

SessionRailViewModel.csUse immutable empty pending set +1/-1

Use immutable empty pending set

• Uses FrozenSet.Empty for the default pending-agent stream instead of allocating a HashSet.

src/Capacitor.App/ViewModels/SessionRailViewModel.cs

TerminalChatInput.csConsolidate terminal send failure handling +0/-2

Consolidate terminal send failure handling

• Lets the existing general exception path handle cancellation as an unconfirmed delivery.

src/Capacitor.App/ViewModels/TerminalChatInput.cs

TerminalTabViewModel.csRemove phantom terminal size fallback +13/-27

Remove phantom terminal size fallback

• Reads the attached surface's actual size directly and removes unused default-size state while tightening lifecycle comments.

src/Capacitor.App/ViewModels/TerminalTabViewModel.cs

TrayViewModel.csUse immutable empty tray collections +37/-42

Use immutable empty tray collections

• Uses FrozenSet.Empty defaults and rewrites stale matrix, command, and composition comments.

src/Capacitor.App/ViewModels/TrayViewModel.cs

WorkContextViewModel.csRemove unused pull-request context flag +0/-2

Remove unused pull-request context flag

• Deletes an unconsumed derived property and its change notification.

src/Capacitor.App/ViewModels/WorkContextViewModel.cs

WorkspaceViewModel.csSimplify terminal banner predicate +1/-1

Simplify terminal banner predicate

• Removes a redundant pull-request-tab check because terminal activity already excludes it.

src/Capacitor.App/ViewModels/WorkspaceViewModel.cs

AppMenuBar.csStop retaining closed report windows +1/-1

Stop retaining closed report windows

• Constructs report menu tracking records without an unused Window reference.

src/Capacitor.App/Views/AppMenuBar.cs

Converters.csCache immutable outcome brushes +6/-4

Cache immutable outcome brushes

• Reuses immutable allowed and denied brushes instead of allocating a mutable brush per conversion.

src/Capacitor.App/Views/Converters.cs

LauncherPaneView.axaml.csReuse launcher brush lookup helper +8/-8

Reuse launcher brush lookup helper

• Replaces repeated resource casts with the existing Brush helper and refreshes picker comments.

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

PullRequestToneBrushConverter.csHide unused tone-key helper +1/-1

Hide unused tone-key helper

• Makes the brush resource-key mapper private because it has no external callers.

src/Capacitor.App/Views/PullRequestToneBrushConverter.cs

ReportItems.csRemove unused report window field +1/-1

Remove unused report window field

• Contracts the report-item record to the two menu items it actually tracks.

src/Capacitor.App/Views/ReportItems.cs

ToolCategoryIcons.csRemove redundant Other icon arm +1/-2

Remove redundant Other icon arm

• Lets Other and unknown categories share the existing default puzzle icon path.

src/Capacitor.App/Views/ToolCategoryIcons.cs

TrayIconRenderer.csRemove redundant attention color arm +2/-3

Remove redundant attention color arm

• Lets attention use the existing disrupted-color default and refreshes renderer comments.

src/Capacitor.App/Views/TrayIconRenderer.cs

Tests (63) +348 / -1404

ActivityStatKeyTests.csCondense activity stat regression rationale +3/-5

Condense activity stat regression rationale

• Retains the missing-rotation-file regression coverage with shorter documentation.

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

ActivityViewModelTests.csPrune duplicate activity projection tests +13/-49

Prune duplicate activity projection tests

• Removes low-value formatting and duplicate visibility-refresh coverage while retaining refresh, polling, failure, and disposal tests.

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

AgentActionServiceTests.csRemove low-value successful stop test +3/-18

Remove low-value successful stop test

• Drops a no-banner happy-path assertion already implied by broader stop tests and refreshes comments.

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

AppMenuBarTests.csRemove duplicate help-menu layout test +0/-8

Remove duplicate help-menu layout test

• Drops a static menu-order assertion while retaining report-item state and click-through coverage.

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

AppMutationLaneWiringTests.csPrune duplicate mutation presentation assertion +1/-14

Prune duplicate mutation presentation assertion

• Removes a known-token mapping test covered by sibling presentation cases and refreshes comments.

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

AppNotifierTests.csCondense notifier concurrency rationale +5/-7

Condense notifier concurrency rationale

• Keeps deterministic channel-order coverage while shortening historical bug context.

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

AppStartupCarveOutTests.csRemove duplicate onboarding gate integration cases +6/-54

Remove duplicate onboarding gate integration cases

• Drops gate cases already covered by OnboardingGateTests and retains app-specific carve-out seams.

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

AppStartupTests.csPrune redundant startup assertions +45/-78

Prune redundant startup assertions

• Removes a duplicate zero-exit shutdown case and shortens extensive historical commentary around retained regressions.

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

AppStateStoreTests.csRemove low-value default-null assertion +0/-7

Remove low-value default-null assertion

• Drops a test that only pinned the default value of the optional harness map.

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

AttachmentDropPasteTests.csCorrect faulted-drop test documentation +2/-2

Correct faulted-drop test documentation

• Describes the tested provider-failure refusal instead of unrelated disabled-sink behavior.

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

AttachmentIntakeTests.csRemove shallow bitmap metadata test +0/-16

Remove shallow bitmap metadata test

• Drops a headless test that only pinned generated filename and content type while retaining byte-limit coverage.

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

AvaloniaSession.csRefresh scheduler helper documentation +2/-3

Refresh scheduler helper documentation

• Removes stale API-migration and acceptance wording from the immediate-scheduler helper.

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

AvaloniaSessionTests.csRemove trivial dispatch return test +0/-7

Remove trivial dispatch return test

• Drops a test that only verified the helper returned a constant value.

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

BootRefusalAttributionTests.csRefresh identity rejection heading +1/-1

Refresh identity rejection heading

• Removes a stale review identifier from the retained rejection cases.

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

ChatTabViewModelTests.csRemove duplicate chat activity tests +0/-41

Remove duplicate chat activity tests

• Drops pending-card and PTY working-note cases covered by named sibling tests.

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

CliResolverTests.csRemove trivial CLI fallback test +0/-7

Remove trivial CLI fallback test

• Drops a one-line fallback assertion already represented by broader path-resolution coverage.

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

ConnectStepViewModelTests.csRemove constant-only connect-step test +0/-11

Remove constant-only connect-step test

• Drops assertions that only pinned fixed identity, applicability, and nonempty title properties.

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

ConsentFlipClaimsTests.csPrune redundant consent claim tests +6/-30

Prune redundant consent claim tests

• Removes duplicate daemon mismatch and storage-path checks while retaining canonicalization, conditional consumption, and quarantine coverage.

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

ConsentFlipCoordinatorTests.csRefresh consent-flip regression comments +6/-6

Refresh consent-flip regression comments

• Keeps coordinator behavior unchanged while replacing stale review and specification references.

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

ConsentPromptCoordinatorTests.csRefresh consent prompt window tests +12/-14

Refresh consent prompt window tests

• Condenses historical defect descriptions around replay, warning visibility, rendering, and shutdown cases.

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

ConsentPromptViewModelTests.csRemove stale consent matrix numbering +16/-16

Remove stale consent matrix numbering

• Retains the complete prompt behavior matrix while simplifying headings and specification-oriented comments.

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

ConsentServiceTests.csRemove stale consent test numbering +13/-13

Remove stale consent test numbering

• Keeps cache, identity, resolution, cancellation, and pruning coverage while simplifying headings.

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

DaemonClientServiceTests.csRefresh daemon client test documentation +8/-8

Refresh daemon client test documentation

• Removes task-history references while retaining lifecycle, reconnect, and mutation-lane assertions.

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

DaemonLifecycleControllerTests.csRefresh lifecycle matrix test rationale +22/-27

Refresh lifecycle matrix test rationale

• Retains startup, reconciliation, mutation routing, and restart coverage with shorter current-state comments.

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

DaemonMutationLaneTests.csRemove duplicate probe-blocking test +0/-20

Remove duplicate probe-blocking test

• Drops a timing case subsumed by the retained pinned-path and executor sequencing coverage.

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

DaemonObservationTests.csRemove low-value pre-slice evidence test +0/-14

Remove low-value pre-slice evidence test

• Drops a fixture-driven assertion that only mirrored the supplied inconsistent probe result.

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

DaemonStepViewModelTests.csRefresh daemon-step matrix documentation +2/-3

Refresh daemon-step matrix documentation

• Describes retained matrix and consent-flip behavior without stale specification references.

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

FakeAppUpdater.csMatch simplified updater test contract +1/-1

Match simplified updater test contract

• Removes the unused progress parameter from the fake updater.

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

FakeClassicDesktopLifetime.csRefresh fake desktop lifetime comment +1/-1

Refresh fake desktop lifetime comment

• Removes a decompiler-specific aside while retaining dispatcher isolation rationale.

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

FakeConsentService.csRefresh consent fake documentation +3/-3

Refresh consent fake documentation

• Removes stale specification references from replay, prune, and resolution semantics.

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

FakeRemoteAgents.csGeneralize remote-agent fake documentation +2/-2

Generalize remote-agent fake documentation

• Describes the fake as reusable by any suite needing AgentDirectory remote rows.

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

HomeViewModelTests.csRefresh home view-model comments +2/-2

Refresh home view-model comments

• Removes obsolete specification and issue references from retained launch and repository tests.

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

HubTestHostTests.csRemove broad hub smoke test +0/-17

Remove broad hub smoke test

• Drops generic invoke-and-broadcast coverage while retaining tests for timing-sensitive host behavior.

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

KcapCliTests.csPrune constant and duplicate CLI tests +3/-25

Prune constant and duplicate CLI tests

• Removes constructor echo and detailed detached-start option checks covered by broader command-building tests.

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

LaunchAttachmentsTests.csRemove pinned attachment version constant test +0/-5

Remove pinned attachment version constant test

• Drops a test that only asserted the minimum daemon version literal.

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

LaunchRequestTests.csClarify production wire-option coverage +2/-5

Clarify production wire-option coverage

• Documents that launch payload serialization uses the same LaunchHubJson naming policy as ServerConnectionService.

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

LifecycleSurfaceTests.csRemove forwarding-only lifecycle tests +5/-41

Remove forwarding-only lifecycle tests

• Drops trivial sink and return forwarding assertions while retaining dialog serialization and cancellation coverage.

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

MacNotificationCategoriesTests.csRemove duplicate category release test +0/-11

Remove duplicate category release test

• Drops the single-owner release case covered by the stronger shared-category lifecycle test.

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

MainWindowSmokeTests.csCondense main-window regression commentary +19/-25

Condense main-window regression commentary

• Retains UI-thread, cancellation, toast, activity, navigation, and rendering coverage with shorter rationale.

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

MainWindowViewModelTests.csRemove unused test fixture construction +6/-19

Remove unused test fixture construction

• Stops creating unused AgentActionService instances and refreshes shell and scheduler comments.

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

MutationRequestFactoryTests.csRefresh mutation factory documentation +1/-1

Refresh mutation factory documentation

• Removes stale task nomenclature from the canonical-server refusal contract.

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

OnboardingGateTests.csRefresh onboarding gate rationale +8/-9

Refresh onboarding gate rationale

• Retains the full gate matrix while removing stale design and review references.

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

OnboardingViewModelTests.csRemove duplicate skip-and-return test +0/-23

Remove duplicate skip-and-return test

• Drops a navigation case whose behavior is covered by broader enter, veto, and direction tests.

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

OutcomeChannelTests.csRefresh transfer requeue comment +1/-1

Refresh transfer requeue comment

• Removes a stale review identifier from retained lease transfer coverage.

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

PauseControllerTests.csRefresh pause controller test comments +4/-5

Refresh pause controller test comments

• Retains strict rule detection, lane ordering, disconnect, and cancellation coverage with direct wording.

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

PendingLaunchRowsTests.csRemove trivial pending-session lookup test +0/-8

Remove trivial pending-session lookup test

• Drops a one-line assertion that pending placeholders do not claim a session.

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

RailSessionViewModelTests.csRemove duplicate failed-status pip test +0/-11

Remove duplicate failed-status pip test

• Drops a status case covered by broader attention and tooltip behavior tests.

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

RecordingAppServices.csRefresh force-confirmation fake comment +1/-1

Refresh force-confirmation fake comment

• Removes stale decision wording from the scripted confirmation seam.

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

RemoteSessionViewModelTests.csExercise proven-twin session mismatch +4/-3

Exercise proven-twin session mismatch

• Marks the local agent as a proven twin so the test actually covers same-agent identity with a different session.

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

ScriptedLocalControlOps.csRemove unused scripted failure helper +1/-2

Remove unused scripted failure helper

• Deletes an uncalled PutV2 exception helper and refreshes cancellation documentation.

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

ServerConnectionServiceTests.csPrune duplicate reconnect integration tests +0/-21

Prune duplicate reconnect integration tests

• Removes generic close-and-restart cases while retaining authenticated connection and shared launch invocation coverage.

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

ShimOfferCoordinatorTests.csRefresh shim offer test comments +6/-6

Refresh shim offer test comments

• Retains platform, suppression, manual retry, and completion coverage without stale task references.

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

TerminalTabViewModelTests.csCondense terminal race rationale +13/-16

Condense terminal race rationale

• Retains actual-size startup, teardown, and reattach race coverage while removing review-history references.

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

TerminalTranscriptTests.csReplace terminal API discovery dump +8/-244

Replace terminal API discovery dump

• Removes extensive decompiler notes and a trivial feed assertion while preserving ANSI, alternate-buffer, input, reply, and reset coverage.

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

TrayAdapterTests.csCondense tray adapter test documentation +19/-25

Condense tray adapter test documentation

• Retains menu, icon, command parameter, enablement, and lifecycle coverage with shorter implementation rationale.

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

TrayViewModelTests.csPrune forwarding and duplicate tray tests +46/-226

Prune forwarding and duplicate tray tests

• Removes command-forwarding and duplicate header cases, and reuses the shared async wait helper.

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

UiTickerTests.csReuse shared ticker wait helper +7/-19

Reuse shared ticker wait helper

• Deletes a local polling helper and condenses the dispatcher-affinity regression explanation.

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

WizardAuthServiceTests.csRemove unused claim fixtures and duplicate arming test +1/-29

Remove unused claim fixtures and duplicate arming test

• Stops constructing irrelevant temporary stores and drops a direct forwarding assertion covered by durable arming cases.

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

WizardCompositionTests.csRefresh wizard composition documentation +1/-1

Refresh wizard composition documentation

• Removes a stale specification reference from the fresh-machine integration scenario.

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

WizardSimpleStepsTests.csPrune constant and forwarding wizard tests +4/-61

Prune constant and forwarding wizard tests

• Removes always-true, no-veto, and exact-label tests while retaining ordered visibility coverage and behavioral cases.

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

WizardStartupTests.csPrune duplicate wizard handoff test +9/-25

Prune duplicate wizard handoff test

• Removes the no-auth handoff case, shares the consent capability constant, and refreshes startup comments.

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

WorkContextViewModelTests.csRemove isolated surrogate-initial test +0/-11

Remove isolated surrogate-initial test

• Drops a narrow presentation assertion from the broader work-context suite.

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

WorkspaceNavigationTests.csRemove duplicate card navigation test +4/-20

Remove duplicate card navigation test

• Drops a direct callback case already covered through rendered card interaction and refreshes lifecycle comments.

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

Documentation (47) +159 / -202

Capacitor.App.csprojRefresh resource packaging comment +1/-1

Refresh resource packaging comment

• Removes a stale specification reference from the product icon resource explanation.

src/Capacitor.App/Capacitor.App.csproj

AgentActionService.csClarify shared agent action behavior +7/-8

Clarify shared agent action behavior

• Removes stale design-document references while preserving stop, notification, and force-confirmation behavior.

src/Capacitor.App/Services/AgentActionService.cs

AppNotifier.csRefresh notifier documentation +2/-2

Refresh notifier documentation

• Retains the replay and stderr semantics while removing obsolete specification references.

src/Capacitor.App/Services/AppNotifier.cs

AppStateStore.csRefresh app-state documentation +1/-1

Refresh app-state documentation

• Removes an obsolete specification citation from the persisted UX-state contract.

src/Capacitor.App/Services/AppStateStore.cs

AttachStatus.csRefresh attach-state documentation +2/-2

Refresh attach-state documentation

• Keeps atomic status and version guarantees while removing stale decision references.

src/Capacitor.App/Services/AttachStatus.cs

ConsentPromptCoordinator.csRefresh prompt coordinator documentation +1/-1

Refresh prompt coordinator documentation

• Clarifies single-window ownership without referring to an external specification.

src/Capacitor.App/Services/ConsentPromptCoordinator.cs

ConsentService.csCorrect consent cache comments +10/-10

Correct consent cache comments

• Rewords request identity, replay, and ABA-guard documentation to describe current behavior directly.

src/Capacitor.App/Services/ConsentService.cs

DaemonClientService.csClarify daemon client lifecycle +8/-11

Clarify daemon client lifecycle

• Shortens stale comments around mutation-lane routing, snapshot ordering, loop containment, and profile binding.

src/Capacitor.App/Services/DaemonClientService.cs

IConsentService.csRefresh consent contract documentation +3/-3

Refresh consent contract documentation

• Describes cancellation, request identity, and prompt visibility without stale specification references.

src/Capacitor.App/Services/IConsentService.cs

IDaemonClientService.csCorrect daemon start contract comment +2/-2

Correct daemon start contract comment

• Documents that starts run through the mutation lane and always trigger a reconnect attempt.

src/Capacitor.App/Services/IDaemonClientService.cs

ILifecycleSurface.csRefresh lifecycle surface documentation +2/-2

Refresh lifecycle surface documentation

• Removes stale specification references from prompt and repair affordance contracts.

src/Capacitor.App/Services/ILifecycleSurface.cs

IPauseController.csRefresh pause controller contract +3/-3

Refresh pause controller contract

• Describes replay, refresh, and queued-toggle behavior without task or specification references.

src/Capacitor.App/Services/IPauseController.cs

KcapCli.csCorrect CLI facade documentation +7/-7

Correct CLI facade documentation

• Clarifies that snapshots expose an app-used subset and that daemon mutations route through the CLI.

src/Capacitor.App/Services/KcapCli.cs

LifecycleSurface.csRefresh lifecycle dialog documentation +4/-6

Refresh lifecycle dialog documentation

• Condenses serialization and cancellation comments while preserving the single-dialog gate.

src/Capacitor.App/Services/LifecycleSurface.cs

NavigationGate.csRefresh navigation gate documentation +1/-1

Refresh navigation gate documentation

• Removes a stale specification citation from the app-lifetime latch explanation.

src/Capacitor.App/Services/NavigationGate.cs

OnboardingGate.csRefresh onboarding gate documentation +3/-3

Refresh onboarding gate documentation

• Explains local gate resolution directly without obsolete decision references.

src/Capacitor.A

[Comment truncated to fit github's 65,536-char limit.]

[qodo-code-review]

Code Review by Qodo

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

1. A tray comment repeats a property name ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

The modified ShimInstallVisible comment only expands the property name into `whether the "Install
command-line tool…" item shows`. Because the record parameter already expresses that meaning, a
later rename or semantic change would require maintaining duplicate text without preserving any
non-obvious constraint.

Code

src/Capacitor.App/ViewModels/TrayModels.cs[49]

+// ShimInstallVisible: whether the "Install command-line tool…" item shows.

Relevance

●●● Strong

Recent precedents accept removing comments that merely paraphrase properties or restate obvious
meaning.

PR-#1063
PR-#1094

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Rule 2762993 rejects comments that duplicate type or signature information. The cited comment
provides only a direct paraphrase of ShimInstallVisible.

Rule 2762993: Restrict comments to documenting non-obvious, behavior‑critical constraints
src/Capacitor.App/ViewModels/TrayModels.cs[49-49]

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 `ShimInstallVisible` comment restates the record parameter name and does not document a non-obvious constraint or rationale.

## Fix Focus Areas
- src/Capacitor.App/ViewModels/TrayModels.cs[49-49]

## Recommended Fix
Delete the redundant comment and let the descriptive parameter name communicate its meaning.

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

---

2. Test suites retain redundant headings ✗ Dismissed 📘 Rule violation ⚙ Maintainability

Description

ActivityViewModelTests, ConsentPromptViewModelTests, ConsentServiceTests, and
DaemonMutationLane retain modified divider comments such as // ---- row mapping ----, `// ----
subscription gate ----, and // --- classification ---` that merely label the tests or methods
below without documenting an invariant or rationale. Because adjacent method names already identify
each section, later reorganization can leave these decorative navigation labels misleading
throughout the files.

Code

test/Capacitor.App.Tests.Unit/ActivityViewModelTests.cs[76]

+    // ---- row mapping ----

Relevance

●●● Strong

Recent test precedents accept removing decorative section banners when test names already identify
the behavior.

PR-#831
PR-#863

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Rule 2762993 permits modified comments only when they capture non-obvious, behavior-critical
information rather than restating code organization. The cited dividers only name the subjects of
the immediately following tests or classification methods and provide no invariant, trade-off,
workaround, trap, constraint, or rationale.

Rule 2762993: Restrict comments to documenting non-obvious, behavior‑critical constraints
test/Capacitor.App.Tests.Unit/ActivityViewModelTests.cs[76-76]
test/Capacitor.App.Tests.Unit/ConsentPromptViewModelTests.cs[19-19]
test/Capacitor.App.Tests.Unit/ConsentServiceTests.cs[32-32]
src/Capacitor.App/Services/Mutation/DaemonMutationLane.cs[278-278]

Agent prompt

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

## Issue description
Several modified divider comments in test suites and `DaemonMutationLane` are only visual headings that repeat subjects already expressed by adjacent method names and provide no non-obvious, behavior-critical rationale.

## Fix Focus Areas
- test/Capacitor.App.Tests.Unit/ActivityViewModelTests.cs[76-404]
- test/Capacitor.App.Tests.Unit/ConsentPromptViewModelTests.cs[19-559]
- test/Capacitor.App.Tests.Unit/ConsentServiceTests.cs[32-442]
- src/Capacitor.App/Services/Mutation/DaemonMutationLane.cs[278-278]

## Recommended Fix
Remove the decorative section-divider comments and rely on descriptive test and classification method names, along with normal code organization, to communicate structure.

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

---

Context sources

✅ Compliance rules (platform): 64 rules

✅ Cross-repo context — repo relationships

  Explored: repo: kurrent-io/kcap-server (sha: f3067f6f)

Review mode: 🧠 Deep: This broad desktop-app audit spans 161 files and 504 independent hunks across startup, services, view models, UI, wire-contract setup, and extensive test pruning, creating many independent opportunities for subtle regressions despite the largely cleanup-oriented intent.

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 ↗

[nortonandreev]

Qodo Fix Summary — Round 1

Reviewed and addressed Qodo review issues:

✅ Fixed Issues

• A tray comment repeats a property name (Medium) - removed the comment that restated ShimInstallVisible

⏭️ Deferred Issues

• Test suites retain redundant headings (Medium) - section-divider comments are the repo-wide convention; removed only the leftover plan numbers (7/8, 12b, 13/14) from three of them
• Split behavioral changes from cleanup (overview suggestion) - the functional removals already sit on the stacked PRs above this one

---

Generated by Qodo PR Resolver skill