Skip to content

Surface 3 (client): report harness inventory to the server - #632

Merged
realtonyyoung merged 3 commits into
mainfrom
claude-tyoung/harness-inventory-surface3
Aug 20, 2026
Merged

realtonyyoung merged 3 commits into
mainfrom
claude-tyoung/harness-inventory-surface3

Conversation

@realtonyyoung

Copy link
Copy Markdown
Collaborator

PR 2 of the new-harness-detection spec (surfaces 1–2 shipped in #626). Attaches a per-machine coding-agent inventory to the two client→server carriers so the kcap-server side (PR 3) can raise the "installed but not configured" UI notification and the machine-went-quiet backstop. This PR is client-only and additive — inert until the server consumes it.

What's here

Core — HarnessInventory { machine_id, vendors: { <vendorId>: { detected, wired } }, declined: [vendorId] } with a pure Evaluate (shared by both carriers), registered in CapacitorJsonContext (snake_case wire shape). machine_id is MachineId.Get() — the same id the daemon already sends on DaemonConnect — so the server correlates a machine's connect record, status-report inventory, and hook-ingest inventory to one machine.

Carrier A — daemon status report: additive nullable field on DaemonStatusReport. AgentOrchestrator caches the inventory and recomputes on a 6h in-memory cadence, deliberately never claiming the on-disk nudge throttle stamp (so it can't starve the hook/CLI nudge surfaces). BuildStatusReport stays pure (reads the cache); the send path refreshes.

Carrier B — hook ingest: SessionStartInventory.Stamp serializes the same HarnessInventory through the same context (→ byte-identical wire shape) onto each vendor's SessionStart body. Wired into all 9 hook commands (guarded to session-start for the shared-injection Claude/Cursor paths). Computed fresh — current-state metadata on an already-happening POST, so no throttle; never throws.

Down-level: additive/nullable — an old server ignores the field; an old client never sends it (the server treats an absent inventory as "unknown", never "nothing detected").

Testing

  • Core Evaluate: vendor→entry mapping + declined list.
  • Daemon: a sent DaemonStatusReport carries the inventory (9 vendors + machine id).
  • Hook: the session-start body carries harness_inventory (integration test, CI-gated like its SessionStartVisibilityTests siblings).
  • AOT publish clean (no IL2026/IL3050); daemon + Core Harness suites green locally.

AI-2118

🤖 Generated with Claude Code

Attaches a per-machine coding-agent inventory to the two client→server carriers
so the server can raise the "installed but not configured" notification and (PR 3)
the machine-went-quiet backstop. This is PR 2 of the new-harness-detection spec;
PR 3 (kcap-server) consumes it.

- Core: HarnessInventory { machine_id, vendors: { <id>: { detected, wired } },
  declined[] } + a pure Evaluate (shared by both carriers), registered in
  CapacitorJsonContext. machine_id is MachineId.Get() — the same id the daemon
  sends on DaemonConnect — so the server correlates a machine's connect record,
  status-report inventory, and hook-ingest inventory.
- Daemon: additive nullable field on DaemonStatusReport; AgentOrchestrator caches
  the inventory and recomputes on a 6h in-memory cadence (never claims the on-disk
  nudge throttle stamp, so surfaces 1–2 aren't starved). BuildStatusReport stays
  pure (reads the cache); the send path refreshes.
- Hook ingest: SessionStartInventory.Stamp serializes the same HarnessInventory
  (via the same context → byte-identical wire shape) onto each vendor's SessionStart
  body; wired into all 9 hook commands (session-start only for Claude/Cursor).
- Additive/down-level-safe: an old server ignores the field; an old client never
  sends it.

Tests: Core Evaluate (mapping + declined); daemon status report carries the
inventory; hook session-start body carries it (integration, CI-gated). AOT clean.

AI-2118
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Client: report harness inventory via daemon status reports and hook session-starts

✨ Enhancement 🧪 Tests 🕐 40+ Minutes

Grey Divider

AI Description

• Add a per-machine HarnessInventory DTO and register it for snake_case JSON serialization.
• Attach cached inventory to daemon status reports, refreshed on a 6-hour in-memory cadence.
• Stamp the same inventory onto hook session-start payloads across all harness vendors, with tests.
Diagram

graph TD
  HookCmds["Hook commands"] --> Stamp["SessionStartInventory"] --> Eval["HarnessInventory.EvaluateCurrent()"] --> Ctx["CapacitorJsonContext"] --> Server{{"kcap-server"}}
  Daemon["Daemon status sender"] --> Cache["AgentOrchestrator cache"] --> Eval --> Report["DaemonStatusReport.harness_inventory"] --> Ctx --> Server
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Server infers inventory from nudge surfaces only
  • ➕ No new wire fields or client-side payload growth
  • ➕ Avoids runtime filesystem/PATH probes on reporting paths
  • ➖ Not reliable when daemon isn’t running and nudges are suppressed/throttled
  • ➖ Harder to correlate to a single machine and to distinguish unknown vs none detected
2. Dedicated inventory endpoint (client pushes on its own cadence)
  • ➕ Clear separation of concerns from status reports and hooks
  • ➕ Server can version the endpoint independently
  • ➖ Adds a new network flow to maintain/auth/retry
  • ➖ More moving parts than leveraging already-sent status reports and session-start requests
3. Compute inventory once at daemon startup and persist to disk
  • ➕ Avoids periodic probes and reduces steady-state overhead
  • ➕ Makes inventory available even if status reporting is delayed
  • ➖ Disk persistence risks staleness and introduces migration/cleanup concerns
  • ➖ Contradicts the intent to avoid claiming/impacting the existing on-disk nudge throttle stamp

Recommendation: The dual-carrier approach in this PR (daemon status report + hook session-start) is the most robust: it preserves down-level compatibility, covers machines without a daemon, and avoids introducing new endpoints. The in-memory 6h TTL is a good tradeoff to limit probe overhead while keeping the status-report path deterministic and non-fatal.

Files changed (16) +226 / -2

Enhancement (13) +140 / -2
Models.csRegister HarnessInventory for JSON source-gen and extend DaemonStatusReport +8/-1

Register HarnessInventory for JSON source-gen and extend DaemonStatusReport

• Adds JsonSerializable registrations for HarnessInventory types in the shared CapacitorJsonContext. Extends DaemonStatusReport with an additive/nullable HarnessInventory field for the daemon→server status-report carrier.

src/Capacitor.Cli.Core/Models.cs

HarnessInventory.csIntroduce HarnessInventory DTO and pure evaluation logic +52/-0

Introduce HarnessInventory DTO and pure evaluation logic

• Adds HarnessInventory and HarnessInventoryEntry records plus a pure Evaluate API shared across carriers. Evaluation covers all HarnessCatalog vendors, includes the machine id, and derives a declined vendor list from the offer ledger, with a convenience EvaluateCurrent() for production.

src/Capacitor.Cli.Core/Setup/HarnessInventory.cs

AgentOrchestrator.csCache and attach harness inventory to daemon status reports +39/-1

Cache and attach harness inventory to daemon status reports

• Introduces an in-memory harness inventory cache with a 6-hour TTL and best-effort refresh that never throws. Refresh runs in the send path, while BuildStatusReport remains pure by reading the cached value and attaching it to DaemonStatusReport.

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

AntigravityHookCommand.csStamp harness inventory onto Antigravity session-start payload +1/-0

Stamp harness inventory onto Antigravity session-start payload

• Adds a SessionStartInventory.Stamp call so Antigravity session-start hook bodies include harness_inventory metadata.

src/Capacitor.Cli/Commands/Harness/AntigravityHookCommand.cs

ClaudeHookCommand.csStamp harness inventory onto Claude session-start payload (only) +4/-0

Stamp harness inventory onto Claude session-start payload (only)

• Stamps harness_inventory only for the session-start command, keeping other events unchanged. Ensures the inventory is reported through the hook carrier without impacting non-session-start events.

src/Capacitor.Cli/Commands/Harness/ClaudeHookCommand.cs

CodexHookCommand.csStamp harness inventory onto Codex session-start payload +1/-0

Stamp harness inventory onto Codex session-start payload

• Adds a SessionStartInventory.Stamp call so Codex session-start hook bodies include harness_inventory metadata.

src/Capacitor.Cli/Commands/Harness/CodexHookCommand.cs

CopilotHookCommand.csStamp harness inventory onto Copilot session-start payload +1/-0

Stamp harness inventory onto Copilot session-start payload

• Adds a SessionStartInventory.Stamp call so Copilot session-start hook bodies include harness_inventory metadata.

src/Capacitor.Cli/Commands/Harness/CopilotHookCommand.cs

CursorHookCommand.csStamp harness inventory onto Cursor sessionStart payload (only) +3/-0

Stamp harness inventory onto Cursor sessionStart payload (only)

• Stamps harness_inventory only for the sessionStart event. Keeps other Cursor events intact while ensuring the session-start carrier reports inventory.

src/Capacitor.Cli/Commands/Harness/CursorHookCommand.cs

GeminiHookCommand.csStamp harness inventory onto Gemini session-start payload +1/-0

Stamp harness inventory onto Gemini session-start payload

• Adds a SessionStartInventory.Stamp call so Gemini session-start hook bodies include harness_inventory metadata.

src/Capacitor.Cli/Commands/Harness/GeminiHookCommand.cs

KiroHookCommand.csStamp harness inventory onto Kiro agent-spawn/session-start payload +1/-0

Stamp harness inventory onto Kiro agent-spawn/session-start payload

• Adds a SessionStartInventory.Stamp call so Kiro’s session-start-equivalent payload includes harness_inventory metadata.

src/Capacitor.Cli/Commands/Harness/KiroHookCommand.cs

OpenCodeHookCommand.csStamp harness inventory onto OpenCode session-start payload +1/-0

Stamp harness inventory onto OpenCode session-start payload

• Adds a SessionStartInventory.Stamp call so OpenCode session-start hook bodies include harness_inventory metadata.

src/Capacitor.Cli/Commands/Harness/OpenCodeHookCommand.cs

PiHookCommand.csStamp harness inventory onto Pi session-start payload +1/-0

Stamp harness inventory onto Pi session-start payload

• Adds a SessionStartInventory.Stamp call so Pi session-start hook bodies include harness_inventory metadata.

src/Capacitor.Cli/Commands/Harness/PiHookCommand.cs

SessionStartInventory.csAdd SessionStartInventory helper to stamp harness inventory into hook bodies +27/-0

Add SessionStartInventory helper to stamp harness inventory into hook bodies

• Introduces a best-effort helper that evaluates HarnessInventory and serializes it via CapacitorJsonContext for byte-identical wire shape with the daemon carrier. Failures are swallowed to ensure hooks never break due to inventory probing.

src/Capacitor.Cli/SessionStartInventory.cs

Tests (3) +86 / -0
HarnessInventoryTests.csUnit test HarnessInventory evaluation and declined vendor behavior +43/-0

Unit test HarnessInventory evaluation and declined vendor behavior

• Adds unit coverage ensuring evaluation populates all nine vendors with detected/wired flags and carries machine id. Verifies declined vendors are listed only when ledger entries are dismissed, and empty otherwise.

test/Capacitor.Cli.Core.Tests.Unit/Setup/HarnessInventoryTests.cs

DaemonStatusReportTests.csVerify sent daemon status reports include harness inventory +16/-0

Verify sent daemon status reports include harness inventory

• Adds a unit test that triggers SendDaemonStatusReportOnceAsync and asserts the captured report includes a non-null HarnessInventory with machine id and nine vendor entries.

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

SessionStartVisibilityTests.csIntegration test: session-start hook payload includes harness_inventory +27/-0

Integration test: session-start hook payload includes harness_inventory

• Adds a CI-gated integration test ensuring the Claude session-start hook request includes a harness_inventory fragment with machine_id and nine vendors, validating the hook carrier wire shape.

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

@qodo-code-review

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

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Remediation recommended

1. TTL bypass on failures ✓ Resolved 🐞 Bug ☼ Reliability
Description
AgentOrchestrator.RefreshHarnessInventoryIfStale() returns on any EvaluateCurrent() exception
without updating the last-evaluated timestamp, so after a failure it will retry on every status
report tick (60s) rather than honoring the intended 6h cadence. This can repeatedly run
filesystem/PATH probes in the hot send loop and still provides no visibility because the exception
is swallowed.
Code

src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[R891-892]

+        try { evaluated = Capacitor.Cli.Core.Setup.HarnessInventory.EvaluateCurrent(); }
+        catch { return; } // keep last cached (or null); inventory must never break the report path
Evidence
The PR adds RefreshHarnessInventoryIfStale() which returns immediately on exceptions, leaving
_harnessInventoryEvaluatedAt unchanged; since the status-report loop calls
SendDaemonStatusReportOnceAsync() every 60 seconds and that method always invokes
RefreshHarnessInventoryIfStale(), any persistent evaluation failure will trigger repeated probing
on each tick instead of the intended 6-hour cadence.

src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[872-896]
src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[1275-1283]
src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[1345-1350]

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

### Issue description
`RefreshHarnessInventoryIfStale()` is intended to recompute the harness inventory “at most once per 6h”, but on exceptions it `return`s before updating `_harnessInventoryEvaluatedAt`. Because `SendDaemonStatusReportOnceAsync()` is called from a 60-second periodic loop, this results in repeated re-evaluation every minute whenever the evaluation throws (including after TTL expiry), potentially doing expensive probes repeatedly and silently.

### Issue Context
- The daemon status report loop runs every 60s and calls `SendDaemonStatusReportOnceAsync()`.
- `SendDaemonStatusReportOnceAsync()` always calls `RefreshHarnessInventoryIfStale()` before sending.
- `RefreshHarnessInventoryIfStale()` swallows all exceptions and currently does not update any timestamp on failure.

### Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[872-896]
- src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[1279-1283]
- src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[1345-1349]

### What to change
1. On evaluation failure, record a backoff timestamp so retries still respect a cadence (e.g., set `_harnessInventoryEvaluatedAt = UtcNow` even when `EvaluateCurrent()` fails, or introduce a separate `_harnessInventoryAttemptedAt` used for throttling attempts).
2. Optionally log the exception at `Debug` (or rate-limited `Warning`) so persistent failures can be diagnosed without breaking the report path.
3. Keep behavior “best-effort”: do not throw; preserve last cached value if any.

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


2. HarnessInventory docs too verbose ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The new HarnessInventory type includes a long comment block that largely documents feature
behavior and wire protocol details rather than focusing on non-obvious rationale, increasing the
risk of stale documentation. This conflicts with the guideline to keep comments minimal and centered
on "why".
Code

src/Capacitor.Cli.Core/Setup/HarnessInventory.cs[R7-10]

+/// <summary>
+/// A machine's coding-agent inventory reported to the server (surface 3): for every supported
+/// vendor, whether it's detected and whether kcap is wired in, plus the vendors the user has
+/// dismissed. The server raises the "installed but not configured" notification for a vendor that
Evidence
PR Compliance ID 11 requires comments to stay short and focused on non-obvious rationale. The added
XML documentation block in HarnessInventory.cs is extensive and includes wire-shape and server
behavior details that go beyond local code intent and are prone to staleness.

CLAUDE.md: Keep Code Comments Minimal and Focused on Non-Obvious 'Why'
src/Capacitor.Cli.Core/Setup/HarnessInventory.cs[7-13]

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

## Issue description
Some newly-added comments are overly verbose and read like embedded spec/protocol documentation rather than concise "why" rationale.

## Issue Context
Rule requires comments to be minimal and focused on non-obvious reasoning; long behavioral/protocol descriptions are more likely to become stale than to help local comprehension.

## Fix Focus Areas
- src/Capacitor.Cli.Core/Setup/HarnessInventory.cs[7-19]
- src/Capacitor.Cli/SessionStartInventory.cs[8-16]
- src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[872-877]

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


Grey Divider

Tip of the day
💡 Did you know, you can copy the agent prompt from any finding and feed it to your IDE agent

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/Capacitor.Cli.Core/Setup/HarnessInventory.cs Outdated
Comment thread src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs Outdated
…Claude spool path

- RefreshHarnessInventoryIfStale now runs check-evaluate-publish under one lock
  (single-flight): overlapping report calls can't each probe, and a slower probe
  can't overwrite a newer one and reset the TTL to stale content. Eval is a few
  dir/PATH stats + a small JSON read, so holding the gate across it is cheap; a
  probe failure no longer advances the timestamp (retries next report). (Codex P2 #1)
- Claude's degraded arm spools the session-start body via NormalizeForSpool and
  bypasses HandleCore's stamp, so a replayed session-start never carried the
  inventory. Stamp session-start bodies in NormalizeForSpool too, so the hook-ingest
  carrier works even for auth/URL-outage replays. (Codex P2 #2)

AI-2118
…bose docs

- RefreshHarnessInventoryIfStale now advances the timestamp on failure too (and logs
  at Debug), so a persistently-failing environment backs off to the 6h TTL instead of
  re-probing on every 60s send. (qodo #2)
- Trimmed the HarnessInventory / SessionStartInventory / status-report-cache doc
  comments to the load-bearing "why" per the repo's comment guideline. (qodo #1)

AI-2118
@realtonyyoung
realtonyyoung merged commit 7345702 into main Aug 20, 2026
6 checks passed
@realtonyyoung
realtonyyoung deleted the claude-tyoung/harness-inventory-surface3 branch August 20, 2026 17:23
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