Hold the evidence scope for the whole run when the server offers it

[realtonyyoung]

AI-3180 (no GitHub issue exists for this change)

What & why

When the catalog's evidence advertisement carries scope_holds: true (kurrent-io/kcap-server#2174), kcap eval and the daemon take their viewer scope with POST evidence-scope/holds under a fresh request_id, renew with POST evidence-scope/renewal instead of resolving again, and release with DELETE evidence-scope/hold when the run's setup is disposed. A held scope does not move when the session grows mid-run, which is what ended long runs as evidence scope moved. A server answering held: false (scope too large to hold) or not advertising holds keeps today's resolve-and-renew behaviour.

Where to look

The hold is owned from the first manifest page that reports it, so a run whose later manifest page fails, or whose preparation fails, still releases it. A renewal that answers another version is a moved scope, as a re-resolve onto another version is today.

Verification

EvidenceScopeHoldClientTests: take carries a 32-char request key; renewal keeps the version and advances the token and deadline; renewal onto another version, 409 or 404 is moved; dispose releases once with the latest token; a failed later page still releases; held: false renews by resolving and releases nothing; no holds advertised sends no hold requests. EvalEvidenceRouteTests: a held run posts its V4 and then releases; a held run that fails to prepare releases. Existing EvidenceScopeClientTests (12) and EvalEvidenceRouteTests (37) pass. AOT publish shows no IL2xxx/IL3xxx warnings.

🤖 Generated with Claude Code

[linear-code]

AI-3180

[qodo-code-review]

PR Summary by Qodo

Hold evidence scopes throughout eval runs when supported

🐞 Bug fix ✨ Enhancement 🧪 Tests 🕐 40+ Minutes

AI Description

• Hold advertised evidence scopes so session growth cannot move their version mid-run.
• Renew held scopes without re-resolving; retain existing behavior when holds are refused or
 unavailable.
• Release holds on completion or preparation failure, with unit and integration coverage.

Diagram

sequenceDiagram
    actor Run
    participant Eval as EvalService
    participant Scope as ScopeClient
    participant API as Evidence API
    participant Setup as RunSetup
    Run->>Eval: Start evidence run
    Eval->>Scope: Resolve advertised scope
    alt Holds advertised
        Scope->>API: POST holds
        API-->>Scope: Manifest and held flag
    else Holds unavailable
        Scope->>API: GET scope
        API-->>Scope: Manifest
    end
    alt Held scope needs renewal
        Scope->>API: POST renewal
        API-->>Scope: Version and new token
    else Unheld scope needs renewal
        Scope->>API: GET scope
        API-->>Scope: Refreshed manifest
    end
    Eval->>Setup: Dispose after run
    Setup->>Scope: Dispose scope
    opt Hold acquired
        Scope->>API: DELETE hold
    end
    Note over Eval,Scope: Preparation failure also disposes scope

Loading

High-Level Assessment

The capability-gated hold lifecycle fits the requirement: re-resolving cannot keep a growing session on one version, while requiring holds would break older servers and refused requests. Keeping acquisition and release in the scope client, with cleanup owned by run setup, preserves the existing fallback.

Files changed (11) +279 / -13

Enhancement (6) +26 / -1

EvalEvidenceAdvertisementDto.csRead the optional scope-holds capability +2/-0

Read the optional scope-holds capability

• Adds the nullable catalog field used to enable holds without changing behavior for older servers.

src/Capacitor.Cli.Core/Eval/Evidence/EvalEvidenceAdvertisementDto.cs

EvidenceScopeHoldCreateRequestDto.csDefine the hold-creation request +11/-0

Define the hold-creation request

• Adds the request ID and scope-selection fields sent when acquiring a hold.

src/Capacitor.Cli.Core/Eval/Evidence/EvidenceScopeHoldCreateRequestDto.cs

EvidenceScopeHoldRequestDto.csDefine the renewal and release request +8/-0

Define the renewal and release request

• Adds the token-bearing request used to renew or release a held scope.

src/Capacitor.Cli.Core/Eval/Evidence/EvidenceScopeHoldRequestDto.cs

EvidenceScopeManifestDto.csRead the manifest's hold result +2/-0

Read the manifest's hold result

• Adds a nullable held flag to distinguish an acquired hold from a refused request or an older response.

src/Capacitor.Cli.Core/Eval/Evidence/EvidenceScopeManifestDto.cs

EvidenceScopeState.csTrack whether the bound scope is held +1/-1

Track whether the bound scope is held

• Adds a held flag so renewal can select the hold endpoint while existing state construction remains compatible.

src/Capacitor.Cli.Core/Eval/Evidence/EvidenceScopeState.cs

Models.csRegister hold requests for JSON source generation +2/-0

Register hold requests for JSON source generation

• Registers both hold request DTOs in the serialization context used by the CLI.

src/Capacitor.Cli.Core/Models.cs

Bug fix (3) +77 / -12

EvalService.Evidence.csSelect holds during preparation and clean up failed preparations +3/-1

Select holds during preparation and clean up failed preparations

• Passes the catalog's hold capability to the scope client and reports a refused hold. Disposes the scope when preparation fails, including after a hold was acquired.

src/Capacitor.Cli.Core/Eval/EvalService.Evidence.cs

EvidenceRunSetup.csRelease the scope when run setup is disposed +6/-2

Release the scope when run setup is disposed

• Disposes the scope before removing the run directory, tying hold release to the prepared run's lifetime.

src/Capacitor.Cli.Core/Eval/Evidence/EvidenceRunSetup.cs

EvidenceScopeClient.csAcquire, renew, and release held scopes +68/-9

Acquire, renew, and release held scopes

• Takes advertised holds with a fresh request ID and tracks the token from the first manifest page. Renews held scopes without re-resolving, releases them on disposal, and retains resolve-based renewal when a hold is refused or unavailable.

src/Capacitor.Cli.Core/Eval/Evidence/EvidenceScopeClient.cs

Tests (2) +176 / -0

EvidenceScopeHoldClientTests.csCover held-scope client behavior and fallbacks +127/-0

Cover held-scope client behavior and fallbacks

• Tests acquisition, renewal, moved-scope responses, latest-token release, and release after a later manifest page fails. Also verifies refused and unavailable holds retain the prior behavior.

test/Capacitor.Cli.Core.Tests.Unit/Eval/Evidence/EvidenceScopeHoldClientTests.cs

EvalEvidenceRouteTests.csVerify hold release across eval-run outcomes +49/-0

Verify hold release across eval-run outcomes

• Adds evidence-route tests confirming release after a completed run and after preparation fails.

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

[qodo-code-review]

Code Review by Qodo

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

1. Parallel questions can end a held run ✗ Dismissed

Description

EnsureScopeAsync lets concurrent callers renew the same held scope using the same captured token,
without serializing renewal or rechecking the updated state. The daemon permits multiple question
handlers to lease one setup, so one renewal can rotate the token while another receives a conflict
that is reported as a run-fatal moved scope.

Code

src/Capacitor.Cli.Core/Eval/Evidence/EvidenceScopeClient.cs[69]

+        if (state.Held) return await RenewAsync(state, ct);

Relevance

●● Moderate

Concurrency correctness concern is plausible, but no closely matching repository precedent
establishes expected renewal serialization.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Question handlers independently lease the same setup and call EnsureScopeAsync. Leases count active
users rather than excluding concurrent users; the new held branch sends a captured token, and a 409
becomes a moved-scope run failure.

src/Capacitor.Cli.Daemon/Services/EvalRunner.cs[187-190]
src/Capacitor.Cli.Daemon/Services/EvalContextCache.cs[106-119]
src/Capacitor.Cli.Core/Eval/EvalService.Evidence.cs[157-160]
src/Capacitor.Cli.Core/Eval/Evidence/EvidenceScopeClient.cs[60-86]
src/Capacitor.Cli.Daemon/Services/EvalRunner.cs[234-240]

Agent prompt

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

## Issue description
Concurrent daemon questions can renew one held scope with the same token, causing a false moved-scope failure.
## Fix Focus Areas
- src/Capacitor.Cli.Core/Eval/Evidence/EvidenceScopeClient.cs[60-87]
## Recommended Fix
Serialize the reopen, deadline check, and renewal for each client; after acquiring the gate, use the latest state so a second caller does not renew an already renewed token.

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

---

2. Daemon threads stall on a slow hold release ✓ Resolved

Description

EvidenceRunSetup.DisposeAsync now awaits Scope.DisposeAsync(), which sends a DELETE with a
10-second timeout, but EvalContextCache.Entry.Release() still waits on disposal with
GetAwaiter().GetResult() on the assumption that it is synchronous. Retirement runs from Live()
on phase lookups, from entry replacement and removal, from the sweep timer and serially for every
entry in Dispose(). A slow or unreachable server can therefore block request handling or the timer
for up to 10 seconds per held run, and shutdown for N times that.

Code

src/Capacitor.Cli.Core/Eval/Evidence/EvidenceRunSetup.cs[R43-46]

+    public async ValueTask DisposeAsync() {
+        await Scope.DisposeAsync();
+        await Context.DisposeAsync();
+    }

Relevance

●●● Strong

Recent daemon precedents accept fixes preventing blocking or incomplete asynchronous teardown.

PR-#438
PR-#479

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The daemon's Release() waits synchronously on setup.DisposeAsync() and its comment assumes
disposal is synchronous. Before this PR disposal only deleted a local directory; it now first calls
the scope client's DisposeAsync, which sends a DELETE with a 10-second timeout.

src/Capacitor.Cli.Daemon/Services/EvalContextCache.cs[160-172]
src/Capacitor.Cli.Core/Eval/Evidence/EvidenceScopeClient.cs[92-101]
src/Capacitor.Cli.Daemon/Services/EvalContextCache.cs[83-96]

Agent prompt

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

## Issue description
Setup disposal now performs an HTTP DELETE (up to 10s), and the daemon cache blocks on it synchronously with `GetAwaiter().GetResult()` from lookup, sweep and shutdown paths.

## Fix Focus Areas
- src/Capacitor.Cli.Core/Eval/Evidence/EvidenceRunSetup.cs[43-46]
- src/Capacitor.Cli.Daemon/Services/EvalContextCache.cs[160-172]

## Recommended Fix
In `Entry.Release()`, run the setup disposal without blocking. For example, start it as a background task that disposes the owned `HttpClient` after the DELETE completes, and log any failure there. Alternatively, make retirement async with a short shared deadline. Update the 'Disposal is synchronous' comment and the log message so they describe the release as well as the directory removal.

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

---

3. Moved renewals can leave holds unreleased ✓ Resolved

Description

RenewAsync returns Moved on a successful response with a different scope version before saving
that response's token in _heldToken. If the renewal advances the hold token while reporting a
changed version, the run exits and disposal sends DELETE with the previous token rather than the
latest one.

Code

src/Capacitor.Cli.Core/Eval/Evidence/EvidenceScopeClient.cs[R84-85]

+        if (page!.ScopeVersion != state.ScopeVersion) return EvidenceScopeStatus.Moved;
+        _heldToken = page.Token;

Relevance

●●● Strong

The stale-token release path is a deterministic cleanup bug directly contradicting the PR’s
latest-token guarantee.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The mismatch check precedes the assignment to _heldToken, and disposal releases only _heldToken.
The renewal tests demonstrate that a successful renewal can return a replacement token and
separately exercise a successful response with a different version.

src/Capacitor.Cli.Core/Eval/Evidence/EvidenceScopeClient.cs[79-86]
src/Capacitor.Cli.Core/Eval/Evidence/EvidenceScopeClient.cs[92-99]
test/Capacitor.Cli.Core.Tests.Unit/Eval/Evidence/EvidenceScopeHoldClientTests.cs[33-50]
test/Capacitor.Cli.Core.Tests.Unit/Eval/Evidence/EvidenceScopeHoldClientTests.cs[54-65]

Agent prompt

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

## Issue description
A successful renewal that reports another version can supply a replacement token, but disposal retains the old token.
## Fix Focus Areas
- src/Capacitor.Cli.Core/Eval/Evidence/EvidenceScopeClient.cs[79-87]
## Recommended Fix
On a successful renewal response, record the returned token for cleanup before returning Moved for a version mismatch. Add a test that disposes after a mismatched-version renewal and checks the released token.

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

---

4. A test comment uses a dated qualifier ✓ Resolved

Description

The new EvidenceScopeHoldClientTests summary calls the fallback today's renewal, tying its
description to when the comment was written rather than to the behavior being tested. A later reader
cannot tell whether today's still describes the resolve-based renewal exercised by the
refused-hold test.

Code

test/Capacitor.Cli.Core.Tests.Unit/Eval/Evidence/EvidenceScopeHoldClientTests.cs[9]

+/// server that refuses or does not offer a hold leaves today's renewal in place.</summary>

Relevance

●●● Strong

Recent test-comment precedents accept removing dated or overly narrative qualifiers from added
comments.

PR-#580
PR-#974

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Rule 2897915 disallows time-sensitive qualifiers in added comments. The added class summary uses
today's for the renewal behavior, while the refused-hold test shows the specific fallback it can
name instead.

Rule 2897915: Avoid time-sensitive or process-reference metadata in code comments
test/Capacitor.Cli.Core.Tests.Unit/Eval/Evidence/EvidenceScopeHoldClientTests.cs[7-9]
test/Capacitor.Cli.Core.Tests.Unit/Eval/Evidence/EvidenceScopeHoldClientTests.cs[95-112]

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 test-class summary uses `today's` to describe a fallback behavior.

## Fix Focus Areas
- test/Capacitor.Cli.Core.Tests.Unit/Eval/Evidence/EvidenceScopeHoldClientTests.cs[7-9]

## Recommended Fix
Replace `today's renewal` with a description of the current behavior, such as renewal by resolving the scope again.

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

---

5. Failed hold releases go unreported ✓ Resolved

Description

EvidenceScopeClient.DisposeAsync ignores the DELETE response status and records transport failures
only in LastError. When release returns a 401, 404 or 5xx response, or the request fails, disposal
continues without any caller checking LastError, leaving the hold until server expiry without an
operator message or log.

Code

src/Capacitor.Cli.Core/Eval/Evidence/EvidenceScopeClient.cs[R96-99]

+            using var request = new HttpRequestMessage(HttpMethod.Delete, Route("hold")) { Content = HoldBody(token) };
+            using var resp    = await http.SendAsync(request, timeout.Token);
+        } catch (Exception e) when (e is HttpRequestException or OperationCanceledException) {
+            LastError = $"could not release the evidence scope hold: {e.Message}";

Relevance

●● Moderate

Unreported release failures threaten observability, but handling policy for best-effort cleanup
failures is context-dependent.

PR-#438
PR-#508

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

DisposeAsync discards the DELETE response without checking IsSuccessStatusCode and does not
propagate caught transport exceptions. DisposeSetupAsync does not read LastError and continues
to run-directory cleanup; the CLI disposal wrapper reports only exceptions it receives, so these
failures are not reported.

src/Capacitor.Cli.Core/Eval/Evidence/EvidenceScopeClient.cs[92-101]
src/Capacitor.Cli.Core/Eval/EvalService.Evidence.cs[352-355]
src/Capacitor.Cli.Core/Eval/Evidence/EvidenceScopeClient.cs[90-101]
src/Capacitor.Cli.Core/Eval/Evidence/EvidenceRunSetup.cs[43-46]
src/Capacitor.Cli.Core/Eval/EvalService.Evidence.cs[351-355]

Agent prompt

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

## Issue description
Hold release can fail through an HTTP error response or a transport exception without being reported to the operator or daemon log.

## Fix Focus Areas
- src/Capacitor.Cli.Core/Eval/Evidence/EvidenceScopeClient.cs[90-101]
- src/Capacitor.Cli.Core/Eval/Evidence/EvidenceRunSetup.cs[43-46]
- src/Capacitor.Cli.Core/Eval/EvalService.Evidence.cs[352-355]

## Recommended Fix
Check the DELETE response's `IsSuccessStatusCode` and record the status code in `LastError` when release fails. After `setup.DisposeAsync()` in `DisposeSetupAsync`, report a set `setup.Scope.LastError` through `observer.OnInfo`, and log release failures in the daemon's release path. Keep failure reporting from preventing run-directory cleanup.

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

---

6. New tests bypass temporary-file helpers ✓ Resolved

Description

The new integration tests call TempRoot() and Claude(), which create directories and files
beneath Tmp using Directory.CreateDirectory and File.WriteAllText instead of TempDir
helpers. Both held-scope tests use these paths, extending the direct filesystem-mutation pattern to
their new temporary state.

Code

test/Capacitor.Cli.Tests.Integration/EvalEvidenceRouteTests.cs[570]

+        var (_, tmp) = TempRoot();

Relevance

● Weak

A recent same-rule precedent rejected refactoring direct temporary filesystem setup to TempDir
helpers.

PR-#974

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Rule 2767472 requires temporary paths beneath a TempDir instance to be created through its helpers.
The added tests invoke TempRoot() and Claude(); those existing helpers directly create
directories and write files beneath paths obtained from Tmp.PathTo.

Rule 2767472: Use TempDir helper for all temporary filesystem state in tests
test/Capacitor.Cli.Tests.Integration/EvalEvidenceRouteTests.cs[567-574]
test/Capacitor.Cli.Tests.Integration/EvalEvidenceRouteTests.cs[75-82]
test/Capacitor.Cli.Tests.Integration/EvalEvidenceRouteTests.cs[119-123]

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 held-scope tests create temporary state through helpers that mutate paths beneath `Tmp` directly.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Integration/EvalEvidenceRouteTests.cs[570-574]
- test/Capacitor.Cli.Tests.Integration/EvalEvidenceRouteTests.cs[588-591]
- test/Capacitor.Cli.Tests.Integration/EvalEvidenceRouteTests.cs[75-82]
- test/Capacitor.Cli.Tests.Integration/EvalEvidenceRouteTests.cs[119-123]

## Recommended Fix
Update the shared test helpers to create directories and files beneath `Tmp` with `TempDir` members, then retain their use in both new tests.

ⓘ 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: b3fea952) — View relationship

Review mode: ⚖️ Balanced: The push changes scope-release lifecycle, async daemon cleanup, token handling, and error propagation across multiple runtime paths, creating real but still reviewable risk in a single careful pass.

Tip of the day

💡 Did you know, you can route each severity your way: inline, summary, both, or drop

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit 9834551

Results up to commit 4a3e822 🧠 Deep

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Parallel questions can end a held run ✗ Dismissed

Description

EnsureScopeAsync lets concurrent callers renew the same held scope using the same captured token,
without serializing renewal or rechecking the updated state. The daemon permits multiple question
handlers to lease one setup, so one renewal can rotate the token while another receives a conflict
that is reported as a run-fatal moved scope.

Code

src/Capacitor.Cli.Core/Eval/Evidence/EvidenceScopeClient.cs[69]

+        if (state.Held) return await RenewAsync(state, ct);

Relevance

●● Moderate

Concurrency correctness concern is plausible, but no closely matching repository precedent
establishes expected renewal serialization.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Question handlers independently lease the same setup and call EnsureScopeAsync. Leases count active
users rather than excluding concurrent users; the new held branch sends a captured token, and a 409
becomes a moved-scope run failure.

src/Capacitor.Cli.Daemon/Services/EvalRunner.cs[187-190]
src/Capacitor.Cli.Daemon/Services/EvalContextCache.cs[106-119]
src/Capacitor.Cli.Core/Eval/EvalService.Evidence.cs[157-160]
src/Capacitor.Cli.Core/Eval/Evidence/EvidenceScopeClient.cs[60-86]
src/Capacitor.Cli.Daemon/Services/EvalRunner.cs[234-240]

Agent prompt

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

## Issue description
Concurrent daemon questions can renew one held scope with the same token, causing a false moved-scope failure.
## Fix Focus Areas
- src/Capacitor.Cli.Core/Eval/Evidence/EvidenceScopeClient.cs[60-87]
## Recommended Fix
Serialize the reopen, deadline check, and renewal for each client; after acquiring the gate, use the latest state so a second caller does not renew an already renewed token.

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

---

2. Daemon threads stall on a slow hold release ✓ Resolved

Description

EvidenceRunSetup.DisposeAsync now awaits Scope.DisposeAsync(), which sends a DELETE with a
10-second timeout, but EvalContextCache.Entry.Release() still waits on disposal with
GetAwaiter().GetResult() on the assumption that it is synchronous. Retirement runs from Live()
on phase lookups, from entry replacement and removal, from the sweep timer and serially for every
entry in Dispose(). A slow or unreachable server can therefore block request handling or the timer
for up to 10 seconds per held run, and shutdown for N times that.

Code

src/Capacitor.Cli.Core/Eval/Evidence/EvidenceRunSetup.cs[R43-46]

+    public async ValueTask DisposeAsync() {
+        await Scope.DisposeAsync();
+        await Context.DisposeAsync();
+    }

Relevance

●●● Strong

Recent daemon precedents accept fixes preventing blocking or incomplete asynchronous teardown.

PR-#438
PR-#479

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The daemon's Release() waits synchronously on setup.DisposeAsync() and its comment assumes
disposal is synchronous. Before this PR disposal only deleted a local directory; it now first calls
the scope client's DisposeAsync, which sends a DELETE with a 10-second timeout.

src/Capacitor.Cli.Daemon/Services/EvalContextCache.cs[160-172]
src/Capacitor.Cli.Core/Eval/Evidence/EvidenceScopeClient.cs[92-101]
src/Capacitor.Cli.Daemon/Services/EvalContextCache.cs[83-96]

Agent prompt

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

## Issue description
Setup disposal now performs an HTTP DELETE (up to 10s), and the daemon cache blocks on it synchronously with `GetAwaiter().GetResult()` from lookup, sweep and shutdown paths.

## Fix Focus Areas
- src/Capacitor.Cli.Core/Eval/Evidence/EvidenceRunSetup.cs[43-46]
- src/Capacitor.Cli.Daemon/Services/EvalContextCache.cs[160-172]

## Recommended Fix
In `Entry.Release()`, run the setup disposal without blocking. For example, start it as a background task that disposes the owned `HttpClient` after the DELETE completes, and log any failure there. Alternatively, make retirement async with a short shared deadline. Update the 'Disposal is synchronous' comment and the log message so they describe the release as well as the directory removal.

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

---

3. Moved renewals can leave holds unreleased ✓ Resolved

Description

RenewAsync returns Moved on a successful response with a different scope version before saving
that response's token in _heldToken. If the renewal advances the hold token while reporting a
changed version, the run exits and disposal sends DELETE with the previous token rather than the
latest one.

Code

src/Capacitor.Cli.Core/Eval/Evidence/EvidenceScopeClient.cs[R84-85]

+        if (page!.ScopeVersion != state.ScopeVersion) return EvidenceScopeStatus.Moved;
+        _heldToken = page.Token;

Relevance

●●● Strong

The stale-token release path is a deterministic cleanup bug directly contradicting the PR’s
latest-token guarantee.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The mismatch check precedes the assignment to _heldToken, and disposal releases only _heldToken.
The renewal tests demonstrate that a successful renewal can return a replacement token and
separately exercise a successful response with a different version.

src/Capacitor.Cli.Core/Eval/Evidence/EvidenceScopeClient.cs[79-86]
src/Capacitor.Cli.Core/Eval/Evidence/EvidenceScopeClient.cs[92-99]
test/Capacitor.Cli.Core.Tests.Unit/Eval/Evidence/EvidenceScopeHoldClientTests.cs[33-50]
test/Capacitor.Cli.Core.Tests.Unit/Eval/Evidence/EvidenceScopeHoldClientTests.cs[54-65]

Agent prompt

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

## Issue description
A successful renewal that reports another version can supply a replacement token, but disposal retains the old token.
## Fix Focus Areas
- src/Capacitor.Cli.Core/Eval/Evidence/EvidenceScopeClient.cs[79-87]
## Recommended Fix
On a successful renewal response, record the returned token for cleanup before returning Moved for a version mismatch. Add a test that disposes after a mismatched-version renewal and checks the released token.

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

---

4. A test comment uses a dated qualifier ✓ Resolved

Description

The new EvidenceScopeHoldClientTests summary calls the fallback today's renewal, tying its
description to when the comment was written rather than to the behavior being tested. A later reader
cannot tell whether today's still describes the resolve-based renewal exercised by the
refused-hold test.

Code

test/Capacitor.Cli.Core.Tests.Unit/Eval/Evidence/EvidenceScopeHoldClientTests.cs[9]

+/// server that refuses or does not offer a hold leaves today's renewal in place.</summary>

Relevance

●●● Strong

Recent test-comment precedents accept removing dated or overly narrative qualifiers from added
comments.

PR-#580
PR-#974

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Rule 2897915 disallows time-sensitive qualifiers in added comments. The added class summary uses
today's for the renewal behavior, while the refused-hold test shows the specific fallback it can
name instead.

Rule 2897915: Avoid time-sensitive or process-reference metadata in code comments
test/Capacitor.Cli.Core.Tests.Unit/Eval/Evidence/EvidenceScopeHoldClientTests.cs[7-9]
test/Capacitor.Cli.Core.Tests.Unit/Eval/Evidence/EvidenceScopeHoldClientTests.cs[95-112]

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 test-class summary uses `today's` to describe a fallback behavior.

## Fix Focus Areas
- test/Capacitor.Cli.Core.Tests.Unit/Eval/Evidence/EvidenceScopeHoldClientTests.cs[7-9]

## Recommended Fix
Replace `today's renewal` with a description of the current behavior, such as renewal by resolving the scope again.

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

---

5. Failed hold releases go unreported ✓ Resolved

Description

EvidenceScopeClient.DisposeAsync ignores the DELETE response status and records transport failures
only in LastError. When release returns a 401, 404 or 5xx response, or the request fails, disposal
continues without any caller checking LastError, leaving the hold until server expiry without an
operator message or log.

Code

src/Capacitor.Cli.Core/Eval/Evidence/EvidenceScopeClient.cs[R96-99]

+            using var request = new HttpRequestMessage(HttpMethod.Delete, Route("hold")) { Content = HoldBody(token) };
+            using var resp    = await http.SendAsync(request, timeout.Token);
+        } catch (Exception e) when (e is HttpRequestException or OperationCanceledException) {
+            LastError = $"could not release the evidence scope hold: {e.Message}";

Relevance

●● Moderate

Unreported release failures threaten observability, but handling policy for best-effort cleanup
failures is context-dependent.

PR-#438
PR-#508

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

DisposeAsync discards the DELETE response without checking IsSuccessStatusCode and does not
propagate caught transport exceptions. DisposeSetupAsync does not read LastError and continues
to run-directory cleanup; the CLI disposal wrapper reports only exceptions it receives, so these
failures are not reported.

src/Capacitor.Cli.Core/Eval/Evidence/EvidenceScopeClient.cs[92-101]
src/Capacitor.Cli.Core/Eval/EvalService.Evidence.cs[352-355]
src/Capacitor.Cli.Core/Eval/Evidence/EvidenceScopeClient.cs[90-101]
src/Capacitor.Cli.Core/Eval/Evidence/EvidenceRunSetup.cs[43-46]
src/Capacitor.Cli.Core/Eval/EvalService.Evidence.cs[351-355]

Agent prompt

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

## Issue description
Hold release can fail through an HTTP error response or a transport exception without being reported to the operator or daemon log.

## Fix Focus Areas
- src/Capacitor.Cli.Core/Eval/Evidence/EvidenceScopeClient.cs[90-101]
- src/Capacitor.Cli.Core/Eval/Evidence/EvidenceRunSetup.cs[43-46]
- src/Capacitor.Cli.Core/Eval/EvalService.Evidence.cs[352-355]

## Recommended Fix
Check the DELETE response's `IsSuccessStatusCode` and record the status code in `LastError` when release fails. After `setup.DisposeAsync()` in `DisposeSetupAsync`, report a set `setup.Scope.LastError` through `observer.OnInfo`, and log release failures in the daemon's release path. Keep failure reporting from preventing run-directory cleanup.

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

---

6. New tests bypass temporary-file helpers ✓ Resolved

Description

The new integration tests call TempRoot() and Claude(), which create directories and files
beneath Tmp using Directory.CreateDirectory and File.WriteAllText instead of TempDir
helpers. Both held-scope tests use these paths, extending the direct filesystem-mutation pattern to
their new temporary state.

Code

test/Capacitor.Cli.Tests.Integration/EvalEvidenceRouteTests.cs[570]

+        var (_, tmp) = TempRoot();

Relevance

● Weak

A recent same-rule precedent rejected refactoring direct temporary filesystem setup to TempDir
helpers.

PR-#974

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Rule 2767472 requires temporary paths beneath a TempDir instance to be created through its helpers.
The added tests invoke TempRoot() and Claude(); those existing helpers directly create
directories and write files beneath paths obtained from Tmp.PathTo.

Rule 2767472: Use TempDir helper for all temporary filesystem state in tests
test/Capacitor.Cli.Tests.Integration/EvalEvidenceRouteTests.cs[567-574]
test/Capacitor.Cli.Tests.Integration/EvalEvidenceRouteTests.cs[75-82]
test/Capacitor.Cli.Tests.Integration/EvalEvidenceRouteTests.cs[119-123]

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 held-scope tests create temporary state through helpers that mutate paths beneath `Tmp` directly.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Integration/EvalEvidenceRouteTests.cs[570-574]
- test/Capacitor.Cli.Tests.Integration/EvalEvidenceRouteTests.cs[588-591]
- test/Capacitor.Cli.Tests.Integration/EvalEvidenceRouteTests.cs[75-82]
- test/Capacitor.Cli.Tests.Integration/EvalEvidenceRouteTests.cs[119-123]

## Recommended Fix
Update the shared test helpers to create directories and files beneath `Tmp` with `TempDir` members, then retain their use in both new tests.

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

---

[realtonyyoung]

Qodo finding 6 (TempDir helpers) fixed in 9834551: the file's Dir/Claude/TempRoot helpers now create through Tmp.CreateDir / TempDirHandle.CreateFile. Findings 2-5 fixed and 1 declined; see the inline replies.

[realtonyyoung]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 9834551