Skip to content

Answer RequestStatusReport2 with a nonce-echoed status report - #718

Merged
realtonyyoung merged 3 commits into
mainfrom
claude-tyoung/ai-2379-correlated-status-reports
Aug 31, 2026
Merged

realtonyyoung merged 3 commits into
mainfrom
claude-tyoung/ai-2379-correlated-status-reports

Conversation

@realtonyyoung

Copy link
Copy Markdown
Collaborator

Closes kurrent-io/kcap-server#1769 — AI-2379

What & why

The server's durable idle-marker pipeline (the daemon_attested wait marker that clears the dashboard's typing indicator) confirms a claim only against a status report echoing its own nonce — and the daemon half of that handshake was never implemented, so every claim died un-confirmed 5s after seeding, for every ACP vendor, every turn. This adds it: a RequestStatusReport2 handler that answers with the one report shape carrying EchoNonce, and SupportsCorrelatedStatusReports: true on connect so the server starts sending the correlated request. The activity clock also fires an out-of-cycle report on a turn's falling edge, cutting indicator latency from the 60s periodic cadence to the server's 30s idle threshold.

Where to look

Unsolicited reports (periodic, delivery-triggered, launch-stage) deliberately never carry a nonce — the superseded no-nonce-member pin in DeliveryTriggeredStatusReportTests is re-pinned as exactly that claim. OnTurnEnded fires on the falling edge only; a turn start emits nothing of its own (the delivered input already reports).

Verification

New CorrelatedStatusReportTests (5): nonce echoed on request, null when unsolicited, snake_case wire tokens pinned through the source-gen context, falling-edge fires exactly one send, clock edge semantics. Daemon suite 2871 total 0 failed; Core suite 2579 total 0 failed; AOT publish clean of IL3050/IL2026.

🤖 Generated with Claude Code

An unsolicited report must leave EchoNonce unset — one carrying a nonce
could masquerade as the answer to a correlated request and confirm an
idle-marker claim the daemon never attested for. Turn end now also fires
an out-of-cycle report, so idleness reaches the server at the edge, not
the next 60s tick.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Aug 31, 2026

Copy link
Copy Markdown

AI-2379

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add nonce-correlated daemon status reports

🐞 Bug fix ✨ Enhancement 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Echo RequestStatusReport2 nonces in status reports to confirm durable idle-marker claims.
• Advertise correlated-report support while keeping unsolicited reports nonce-free.
• Emit immediate status reports when agent turns end, reducing idle-detection latency.
Diagram

sequenceDiagram
    participant Server as Server Hub
    participant Connection as Server Connection
    participant Orchestrator as Agent Orchestrator
    participant Clock as Activity Clock
    participant Report as Status Report
    Connection->>Server: Connect with capability
    Server->>Connection: RequestStatusReport2 nonce
    Connection->>Orchestrator: Dispatch nonce
    Orchestrator->>Report: Build with echo nonce
    Report->>Server: Send correlated report
    Clock->>Orchestrator: Turn-ended notification
    Orchestrator->>Report: Build nonce-free report
    Report->>Server: Send idle update
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Separate correlated response DTO
  • ➕ Makes nonce-bearing responses structurally distinct from unsolicited reports.
  • ➕ Prevents accidental nonce reuse through the type system.
  • ➖ Requires another server and daemon wire contract.
  • ➖ Duplicates the existing status payload and increases rollout complexity.
  • ➖ Does not match the server's ordinary status-report ingestion path.

Recommendation: Keep the PR's additive fields and shared report shape. The capability gate preserves mixed-version compatibility, while defaulting EchoNonce to null keeps every existing unsolicited path safe; focused tests pin the boundary that a separate DTO would enforce more rigidly but at substantially higher protocol cost.

Files changed (6) +168 / -23

Enhancement (1) +10 / -0
AgentActivityClock.csSignal agent turn falling edges +10/-0

Signal agent turn falling edges

• Adds an OnTurnEnded callback that fires only when a turn transitions from in-flight to idle. Invocation occurs outside the clock lock to avoid callback reentrancy and deadlock risks.

src/Capacitor.Cli.Daemon/Services/AgentActivityClock.cs

Bug fix (3) +36 / -10
Models.csExtend status-report contracts with nonce correlation +16/-2

Extend status-report contracts with nonce correlation

• Adds the source-generated StatusReportRequest contract, an optional EchoNonce on daemon reports, and a connect-time correlated-report capability flag. Trailing defaults preserve compatibility with older daemon and server versions.

src/Capacitor.Cli.Core/Models.cs

AgentOrchestrator.csBuild correlated and turn-ended status reports +12/-7

Build correlated and turn-ended status reports

• Routes RequestStatusReport2 nonces into the report builder and sender while preserving null nonces for unsolicited emissions. Wires activity-clock turn endings to immediate out-of-cycle status sends.

src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs

ServerConnection.csHandle correlated status requests and advertise support +8/-1

Handle correlated status requests and advertise support

• Registers the RequestStatusReport2 SignalR handler, offloads it through the existing safe invocation pattern, and exposes the request nonce to the orchestrator. Advertises correlated status-report support during daemon connection.

src/Capacitor.Cli.Daemon/Services/ServerConnection.cs

Tests (2) +122 / -13
CorrelatedStatusReportTests.csCover nonce correlation and turn-end reporting +109/-0

Cover nonce correlation and turn-end reporting

• Adds tests for echoed and absent nonces, pinned snake_case wire names, orchestrator turn-end sends, and falling-edge-only clock callbacks.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/CorrelatedStatusReportTests.cs

DeliveryTriggeredStatusReportTests.csVerify delivery-triggered reports remain uncorrelated +13/-13

Verify delivery-triggered reports remain uncorrelated

• Replaces the obsolete reflection assertion with a behavioral test proving delivery-triggered status reports leave EchoNonce unset.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/DeliveryTriggeredStatusReportTests.cs

@qodo-code-review

qodo-code-review Bot commented Aug 31, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Action required

1. Inline comments narrate assertions ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The new turn-edge test adds inline comments that merely restate each state transition and the
immediately following assertion. These comments add no non-obvious constraint and violate the
requirement to keep comments constraint-focused.
Code

test/Capacitor.Cli.Daemon.Tests.Unit/Services/CorrelatedStatusReportTests.cs[63]

+        agent.ActivityClock.SetTurnInFlight(true); // turn START: no out-of-cycle send
Evidence
Rule 20 prohibits comments that merely restate code. The comment on line 63 says a turn start causes
no send, while that same line performs the turn start and lines 64-65 immediately verify zero sends;
the same narration pattern recurs for the falling edge and repeated false assignment.

CLAUDE.md: Write Only Current, Constraint-Focused Comments
test/Capacitor.Cli.Daemon.Tests.Unit/Services/CorrelatedStatusReportTests.cs[63-73]

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 inline comments in the turn-edge test narrate behavior already expressed by the method calls and assertions.

## Issue Context
PR Compliance ID 20 permits comments for non-obvious constraints, but rejects comments that merely restate code. Keep the test method and assertions unchanged unless needed for clarity.

## Fix Focus Areas
- test/Capacitor.Cli.Daemon.Tests.Unit/Services/CorrelatedStatusReportTests.cs[63-71]

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


Grey Divider

Context sources
Review mode: ⚖️ Balanced

Grey Divider

Tip of the day
💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread test/Capacitor.Cli.Daemon.Tests.Unit/Services/CorrelatedStatusReportTests.cs Outdated
realtonyyoung and others added 2 commits August 31, 2026 15:43
Only the repeated call needs a note: nothing else distinguishes it from the
line above.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The receive seam answers through a null-conditional invoke, so an unwired
connection claiming support invites requests it meets with silence.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@realtonyyoung

Copy link
Copy Markdown
Collaborator Author

Codex review flow — two P1s.

Capability advertised without a live handler — fixed in 4d5407e. The connect payload now reads AdvertisesCorrelatedStatusReports, which is the handler itself, so an unwired connection (early startup, a test, a second ServerConnection) cannot invite RequestStatusReport2 frames that its null-conditional invoke would answer with silence. Pinned by An_unwired_connection_does_not_advertise_correlated_status_reports.

Falling-edge snapshot — not taking this one. The suggested remedy, carrying the edge's captured state into the queued report, is the exact thing _statusReportOrderingGate exists to prevent: BuildStatusReport() is evaluated inside the section precisely so a snapshot taken before acquisition never reaches the wire. And in the race described, the report is not wrong — a turn that fell and rose again is genuinely in flight, and TurnInFlight: true is the truth at send time. What is lost is a momentary idle blip the server would have to act on within the gate wait; the next tick reconciles. Reporting a level that is no longer true, to attest an edge, would be the worse trade.

@realtonyyoung
realtonyyoung merged commit fda926c into main Aug 31, 2026
6 checks passed
@realtonyyoung
realtonyyoung deleted the claude-tyoung/ai-2379-correlated-status-reports branch August 31, 2026 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant