[George-Payne]

AI-2307

What & why

The flow's import-outcome route folded a report into FirstRunFlowState.ImportOutcome and had no
caller, so the Done screen's counts caption was unreachable — and since the outcome is also the signal
that the run finished, the screen cannot tell a working import from a stalled one. This is the caller.
The route itself is still open as kcap-server#1671, so this is written against the contract on that
branch and degrades on a 404 like any other newer-server route.

Where to look

• Null is not (0,0,0). A run that lost a pass reports nothing — its sessions are unaccounted and
  three counts cannot say so, while the surviving pass's figures alone would state a clean import. An
  empty Choices likewise has two causes, and IsDecline separates a real decline from a decision
  whose every level was unreadable here.
• The held report's retry is deliberately uncredited against the poll budget, unlike the scan and
  the import: it runs every tick for as long as the report is refused, so crediting it would let a
  server that never accepts it stretch the 30-minute backstop into hours.
• Status retry is opt-in per call site — every hook, watch, daemon and MCP path shares
  SendWithRetryAsync and their budgets assume one attempt. An exhausted budget returns the status
  rather than throwing, because those call sites catch HttpRequestException only.

Verification

Core 2459, Cli 3727, App 1161 — all pass. dotnet publish -c Release clean, no IL2026/IL3050. The
daemon suite fails 12–29 in this container on both this branch and an untouched origin/main worktree
(three runs each, overlapping ranges): contention between subprocess-heavy classes, left to CI.

Sixteen mutations, fifteen killed and one shown equivalent — swapping answer.DecidedAt for
view.ImportDecidedAt, which are the same value by construction. The retry predicate is pinned from
both sides: retrying a 4xx kills six tests, not retrying a 429 kills two, defaulting the flag on kills
the opt-in test.

The wire shape is round-tripped through the source-generated context against WireMock, present and
absent. Nothing else covered it — every other test in that file builds the request object directly, so
a naming or AOT-binding slip would leave the screen waiting for ever with the suite green.

[linear-code]

AI-2307

[qodo-code-review]

PR Summary by Qodo

Report first-run import outcomes and retry transient upload failures

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

AI Description

• Reports import totals and refusal reasons so the Done screen detects completion.
• Preserves unknown outcomes while retrying refused reports without extending poll deadlines.
• Retries transient upload statuses and adds comprehensive flow, wire, and retry coverage.

Diagram

sequenceDiagram
    participant Flow as First-run flow
    participant Lane as Import lane
    participant Command as Import command
    participant Retry as HTTP retry
    participant Client as Flow client
    participant API as Server APIs
    participant Screen as Done screen
    Flow->>Lane: Act on decision
    Lane->>Command: Run import passes
    Command->>Retry: Upload sessions
    Retry->>API: Send counted writes
    API-->>Retry: Status response
    Retry-->>Command: Final result
    Command-->>Lane: Totals or null
    alt Accounted or refused
        Lane->>Client: Report held outcome
        Client->>API: POST import outcome
        API-->>Client: Accepted or refused
        Client-->>Lane: Recorded status
        opt Outcome refused
            Flow->>Lane: Retry next tick
            Lane->>Client: Resend same outcome
        end
        API-->>Screen: Completion and counts
    else Pass unaccounted
        Lane-->>Flow: Leave outcome unset
    end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Generalized durable report outbox

• ➕ Could unify discovery, action, and import-outcome delivery guarantees.
• ➕ Could survive CLI process termination between import completion and reporting.
• ➖ Requires broader persistence and lifecycle design beyond this first-run bug.
• ➖ Adds migration, cleanup, and deduplication complexity for a short-lived flow.

2. Derive outcomes server-side

• ➕ Avoids a new client reporting path.
• ➕ Centralizes Done-screen state generation.
• ➖ The server cannot observe local no-op decisions, unreadable choices, or visibility-preflight losses.
• ➖ Upload events alone cannot distinguish a stalled client from a completed zero-result run.

3. Enable status retries globally

• ➕ Simplifies call-site configuration.
• ➕ Makes transient-status handling uniform across all HTTP consumers.
• ➖ Changes latency and retry budgets for hooks, watches, daemon, and MCP paths.
• ➖ May duplicate non-idempotent requests or delay callers designed for one attempt.

Recommendation: Keep the PR's scoped approach: retain the owed outcome in the import lane, report the exact decision stamp, and opt status retries in only for counted idempotent writes. A durable outbox could be reconsidered if outcomes must survive process restarts, but server-side derivation and global retries cannot preserve the current semantics safely.

Files changed (16) +1250 / -81

Enhancement (5) +130 / -6

FirstRunFlowClient.csPost import outcomes to the first-run API +29/-2

Post import outcomes to the first-run API

• Extends the flow channel with import-outcome reporting and serializes the new request to the decision-specific endpoint. Transport failures degrade to an unrecorded result for later flow-level retry.

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

FirstRunFlowModels.csDefine the import outcome wire request +25/-0

Define the import outcome wire request

• Adds the required decision timestamp and imported, skipped, failed, and optional reason fields matching the server route contract.

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

FirstRunImportOutcome.csDefine import totals and refusal reason tokens +44/-0

Define import totals and refusal reason tokens

• Introduces additive import totals plus the server-approved tokens for unreadable decisions and unavailable agents.

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

IFirstRunImportLane.csReturn measured totals from import lane executions +10/-1

Return measured totals from import lane executions

• Changes the import contract to return aggregate totals or null when any pass lacks trustworthy accounting.

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

SetupCommand.csAggregate first-run totals across import passes +22/-3

Aggregate first-run totals across import passes

• Sums loaded, resumed, skipped, failed, and visibility-preflight losses across private and shared passes. Returns null if any pass throws or fails to produce accounting.

src/Capacitor.Cli/Commands/SetupCommand.cs

Bug fix (3) +260 / -73

BrowserFirstRunFlow.csDeliver decision-correlated outcomes from the import lane +100/-13

Deliver decision-correlated outcomes from the import lane

• Builds success, clean-zero, and refusal reports; retains refused outcomes across poll ticks; and flushes them before a finished flow exits. Unreadable decisions advance the cursor, while runs with unaccounted passes intentionally remain unreported.

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

HttpClientExtensions.csAdd bounded opt-in retries for transient HTTP statuses +132/-56

Add bounded opt-in retries for transient HTTP statuses

• Retries 408, 429, and 5xx responses when requested, honors Retry-After within the remaining budget, and returns the latest refusal when retries expire. Default behavior remains one completed response attempt for existing callers.

src/Capacitor.Cli.Core/HttpClientExtensions.cs

ImportCommand.csReport measured zero runs and retry counted writes +28/-4

Report measured zero runs and retry counted writes

• Invokes completion callbacks on no-agent, no-transcript, and empty-scope exits so zero is distinguishable from missing accounting. Enables transient-status retries for transcript hooks and visibility writes whose failures affect reported totals.

src/Capacitor.Cli/Commands/ImportCommand.cs

Tests (5) +654 / -2

BrowserFirstRunFlowTests.csCover import outcome orchestration and delivery +218/-2

Cover import outcome orchestration and delivery

• Tests totals, refusal shapes, clean declines, null outcomes, decision correlation, repeated delivery, final flushing, unreadable-decision cursoring, and poll deadline enforcement.

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

FirstRunFlowClientTests.csVerify import outcome wire serialization +89/-0

Verify import outcome wire serialization

• Exercises the real HTTP client against WireMock to verify JSON field names, timestamps, refusal tokens, and non-success result handling.

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

HttpClientExtensionsRetryTests.csCover transient status retry edge cases +196/-0

Cover transient status retry edge cases

• Tests retryable and terminal statuses, opt-in defaults, Retry-After limits, budget exhaustion, cancellation, late transport faults, and response ownership.

test/Capacitor.Cli.Core.Tests.Unit/HttpClientExtensionsRetryTests.cs

ImportVisibilityTests.csVerify empty-scope imports emit measured zero totals +31/-0

Verify empty-scope imports emit measured zero totals

• Confirms an import with no sessions in scope still invokes the completion callback with a successful zero outcome.

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

SetupImportLaneTests.csVerify import pass aggregation and unknown accounting +120/-0

Verify import pass aggregation and unknown accounting

• Covers multi-pass sums, resumed and probe-error classification, visibility failures, measured zeroes, and null results for thrown or unreported passes.

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

Documentation (2) +205 / -0

CHANGES.mdDocument import outcome delivery and retry semantics +46/-0

Document import outcome delivery and retry semantics

• Explains the completed outcome-reporting flow, null-versus-zero accounting contract, unreadable-decision handling, and bounded opt-in status retries.

docs/CHANGES.md

2026-08-27-ai2307-import-outcome-report-design.mdAdd the AI-2307 import outcome design specification +159/-0

Add the AI-2307 import outcome design specification

• Records the route contract, count derivation, refusal reasons, retry behavior, correlation guarantees, and verification strategy for the feature.

docs/superpowers/specs/2026-08-27-ai2307-import-outcome-report-design.md

Other (1) +1 / -0

Models.csRegister the import outcome DTO for source generation +1/-0

Register the import outcome DTO for source generation

• Adds the new request type to the source-generated JSON serialization context for AOT-safe wire binding.

src/Capacitor.Cli.Core/Models.cs

[qodo-code-review]

Code Review by Qodo

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

1. Manual temp path construction ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

The new test builds a throwaway path with Path.Combine(_tempDir, ...) instead of the shared
TempDir.PathTo API. This bypasses the required portable temp-directory abstraction.

Code

test/Capacitor.Cli.Tests.Unit/Commands/ImportVisibilityTests.cs[537]

+        var projectsDir = Path.Combine(_tempDir, "claude-projects-nothing-in-scope");

Evidence

PR Compliance ID 13 requires throwaway test paths to use TempDir.PathTo; the changed test instead
calls Path.Combine on the raw temp path, while the shared helper exposes PathTo.

CLAUDE.md: Tests Must Use the Shared TempDir APIs and Correct Lifetime Model
test/Capacitor.Cli.Tests.Unit/Commands/ImportVisibilityTests.cs[537-538]
test/Capacitor.Tests.Helpers/TempDir.cs[26-26]

Agent prompt

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

## Issue description
The test manually combines a path under a `TempDir` instead of using the shared temp-directory API.

## Issue Context
`ImportVisibilityTests` already owns `_tmp` as a `TempDir`; construct the child path through `_tmp.PathTo(...)` and remove reliance on the raw `_tempDir` alias where possible.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Unit/Commands/ImportVisibilityTests.cs[537-538]

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

---

2. Task 8 comment preserved ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

Adding the new JSON registration leaves the adjacent Task 8 planning coordinate untouched in the
modified registration block. The touched-comment rule requires nearby task narration to be
de-historicized.

Code

src/Capacitor.Cli.Core/Models.cs[979]

+[JsonSerializable(typeof(FirstRun.ReportFirstRunImportOutcomeRequest))]

Evidence

PR Compliance ID 26 requires comments around touched code to be stripped of prohibited historical or
planning narration. The added registration at line 979 touches a block whose immediately adjacent
comment labels the following DTO registrations as Task 8.

CLAUDE.md: Touched Comments Must Be Shortened and De-Historicized
src/Capacitor.Cli.Core/Models.cs[979-982]

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 modified source-generation registration block retains an adjacent `Task 8` project-planning comment.

## Issue Context
The comment is immediately beside the added registration and uses prohibited task-history narration. Remove it or replace it only if a short, timeless runtime constraint genuinely needs documenting.

## Fix Focus Areas
- src/Capacitor.Cli.Core/Models.cs[979-982]

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

---

3. Direct ValueKind null check ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

The new wire-format test directly inspects reason.ValueKind even though
JsonElementExtensions.IsNull provides this operation. This violates the required centralized JSON
inspection style.

Code

test/Capacitor.Cli.Core.Tests.Unit/FirstRun/FirstRunFlowClientTests.cs[599]

+        await Assert.That(reason.ValueKind).IsEqualTo(System.Text.Json.JsonValueKind.Null);

Evidence

PR Compliance ID 27 forbids changed code from directly checking ValueKind when a repository
extension supports the operation; JsonElementExtensions defines IsNull, but line 599 compares
against JsonValueKind.Null directly.

CLAUDE.md: Code Must Use JsonElementExtensions for JSON Value Inspection
test/Capacitor.Cli.Core.Tests.Unit/FirstRun/FirstRunFlowClientTests.cs[598-599]
src/Capacitor.Cli.Core/JsonElementExtensions.cs[16-18]

Agent prompt

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

## Issue description
The test directly compares a `JsonElement.ValueKind` instead of using the repository JSON inspection extension.

## Issue Context
`JsonElementExtensions` defines `IsNull` for an element already in hand. Replace the direct `JsonValueKind.Null` assertion with an assertion over `reason.IsNull`.

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

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

---

View high (1)

4. Review metadata committed to spec ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

The added design document records external-review findings, mutation counts, and mutation-review
history. These are transient review artifacts explicitly prohibited from repository comments and
documentation.

Code

docs/superpowers/specs/2026-08-27-ai2307-import-outcome-report-design.md[R149-151]

+Sixteen mutations, fifteen killed and one shown to be equivalent (the correlation swap above). The retry
+predicate is pinned from both sides: retrying a 4xx kills six tests, not retrying a 429 kills two, and
+defaulting the flag to on kills the opt-in test.

Evidence

PR Compliance ID 25 prohibits reviewer references, PR findings, transient counts, and change/review
narration. The cited section records Sixteen mutations, kill counts, and later attributes defects
to external review.

CLAUDE.md: Comments Must Exclude Historical and Review Metadata Except for Defined Live Constraints
docs/superpowers/specs/2026-08-27-ai2307-import-outcome-report-design.md[149-159]

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 design document includes mutation totals, external-review findings, and review-process narration that will become stale.

## Issue Context
Retain only timeless behavioral constraints needed to understand the implementation. Remove transient counts, reviewer references, mutation-aim history, and similar validation narration.

## Fix Focus Areas
- docs/superpowers/specs/2026-08-27-ai2307-import-outcome-report-design.md[149-159]

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

---

Context sources

Review mode: ⚖️ Balanced

Tip of the day

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

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[realtonyyoung]

NO FINDINGS

[realtonyyoung]

NO FINDINGS