Skip to content

feat(cloud-agent-next): collect deduplicated outcome aggregates - #6213

Merged
eshurakov merged 2 commits into
mainfrom
eshurakov/frosty-lark
Sep 16, 2026
Merged

eshurakov merged 2 commits into
mainfrom
eshurakov/frosty-lark

Conversation

@eshurakov

Copy link
Copy Markdown
Contributor

Summary

Adds the first piece of Cloud Agent health monitoring: a scheduled collector that computes deduplicated settled-turn outcomes from the Cloud Agent reporting tables and emits one small structured record per window into Axiom via the existing Logpush pipeline.

Why: the only outcome view today is the admin health overview, which computes failed / (completed + failed) over run rows with no runtime-generation split and no failure-stage breakdown. There was no deduplicated per-turn outcome metric, and no way to see which lifecycle stage failed (pre-dispatch, no-activity, after-activity).

What it collects, per runtime generation and per turn role, over half-open terminal_at windows of 5/15/60 minutes on the 90-day retained population:

  • completed / platform / user / unknown / interrupted counts, settled, and platformFailureShare / unknownClassificationShare (null, not 0, on an empty denominator)
  • distinct affected sessions at generation level — one session retrying N turns counts once
  • failure composition by failure_stage and (failure_stage, failure_code)
  • session-setup failures as a separate population, never summed into settled

Design notes:

  • The three queries run sequentially in one read-only REPEATABLE READ transaction, so a window is a consistent snapshot; emission happens after the transaction returns.
  • A collection failure emits a collectionStatus: 'failed' record with no counts — never a zero snapshot.
  • Generation has no column; it is derived from the session-id prefix (workspace_ => control, else legacy), with the prefix exported from session-plane.ts as the single owner.
  • Turn role is exact identity (message_id = initial_message_id), not ordering or dispatch state.
  • The retention boundary reuses the existing retentionCutoff owner, and the coverage label derives from the retention constant.
  • The record reports categories it observed; it does not prove a message never reached the model (see reviewer notes).

Wiring:

  • scheduled() now dispatches by exact cron identity. Retention stays 17 2 * * *; collection is provisionally */3 * * * *. Unrecognized expressions warn and do nothing, so no fallback can run retention at the wrong cadence.
  • Adds a real-PostgreSQL Vitest suite (test:postgres) plus a CI Postgres service; the existing suite mocks the database and therefore cannot verify the aggregate SQL.

Out of scope: Axiom monitor/dashboard definitions, alert delivery, the PR 1 log-signal catalog, the PR 3 freshness signal, and PR 4 progress facts.

Verification

Manually exercised against a locally running Worker (real wrangler dev --env dev --test-scheduled against a migrated local Postgres, serving the uncommitted changes):

  • */3 * * * * => exactly one collection: one aggregate record per window (5/15/60), collectionStatus: 'complete', counts matching the seeded rows (completed 1, platformFailed 2, settled 3, distinctPlatformAffectedSessions 1, stage/code breakdown present, 90-day-expired session excluded).
  • Negative control: the previous * * * * * expression now hits the unrecognized-cron branch (warn only, no collection, no retention), proving the running Worker uses the new constant.
  • 17 2 * * * => retention only; the expired session is deleted and no aggregate record is emitted.
  • Unknown cron => neither path runs; warn logged.

Visual Changes

N/A

Reviewer Notes

  • The cron dispatch is the highest-risk part. scheduled() previously ran retention unconditionally. A drift test asserts the root and env.dev cron arrays equal exactly the exported constants. Rollback order matters: remove the collection trigger and keep the dispatcher deployed until cron deletion propagates, then revert code.
  • Cadence is provisional. Production query cost is not measured; each tick issues 9 indexed SELECTs (3 windows x 3 queries). Changing cadence requires updating the dispatcher constant and both Wrangler trigger lists together.
  • The record reports categories, not proof. For the control plane, post_dispatch_no_activity does not establish prior activity, so it must not be read as proof that a message never reached the model. The emitted limitations[] labels carry this.
  • environment is null, so consumers must scope evaluations by the deployed Worker identity (dataset plus Worker name), not by the constant service field.
  • The freshness threshold must be derived from cadence plus measured delay when the PR 3 collector-freshness monitor is built; the source plan document still states a 1-minute cadence and a 5-minute threshold and needs reconciling.
  • No alert destination is decided yet (native Axiom monitors vs. the existing o11y stack), and nothing pages after this PR.

…egates

Add a scheduled collector that computes deduplicated settled-turn outcomes
from the Cloud Agent reporting tables and emits one small structured
record per window into Axiom through the existing Logpush pipeline.

- Half-open terminal-time windows (5/15/60 min) over the 90-day retained
  reporting population, joined to the parent session.
- Splits by runtime generation (session-id prefix) and turn role (initial
  vs follow-up); platform/unknown failure shares are null, never zero,
  when the denominator is empty.
- Reports failure_stage/failure_code composition and session-setup
  failures as a separate population; interrupted turns never enter
  settled.
- Runs the three queries sequentially in one read-only REPEATABLE READ
  transaction so a window is a consistent snapshot; a failed collection
  emits a failed-status record instead of zeros.
- Dispatches the Worker scheduled handler by exact cron identity so a new
  collection trigger can never run the daily 90-day retention sweep.
  Retention stays on 17 2 * * *; collection is provisionally */3 * * * *.
- Adds a real-Postgres Vitest suite (test:postgres) with a CI Postgres
  service, covering dedup/merge, half-open boundaries, retention cutoff,
  absent responsibility, generation agreement, and snapshot coherence.

The collector only exposes reported categories; it does not prove that a
message never reached the model. Axiom monitor definitions, alert
delivery, and the PR 3 freshness signal are out of scope.
Comment thread services/cloud-agent-next/src/telemetry/outcome-aggregate.ts Outdated
@kilo-code-bot

kilo-code-bot Bot commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental change verifies cleanly: both null failure_code coalesces now emit the established 'unclassified' sentinel, matching the unknown:unclassified pair in the report classification catalog and the existing COALESCE(..., 'unclassified') precedent, and the added PostgreSQL case pins that behavior with no current findings.

Files Reviewed (3 files)
  • services/cloud-agent-next/src/telemetry/outcome-aggregate.ts
  • services/cloud-agent-next/src/telemetry/outcome-aggregate.test.ts
  • services/cloud-agent-next/test/postgres/outcome-aggregate.postgres.test.ts
Previous Review Summary (commit 498aacf)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 498aacf)

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

The scheduled collection and cron-dispatch changes are sound; the only issue is a null failure_code coalescing to 'unknown' instead of the established 'unclassified' sentinel in the emitted aggregate record.

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1
Issue Details (click to expand)

SUGGESTION

File Line Issue
services/cloud-agent-next/src/telemetry/outcome-aggregate.ts 102 coalesce(failure_code, 'unknown') emits a value outside CloudAgentSessionRunFailureCode; the established sentinel for a null code is 'unclassified'.
Files Reviewed (12 files)
  • .github/workflows/ci.yml - no issues
  • services/cloud-agent-next/package.json - no issues
  • services/cloud-agent-next/src/server.test.ts - no issues
  • services/cloud-agent-next/src/server.ts - no issues
  • services/cloud-agent-next/src/session-plane.ts - no issues
  • services/cloud-agent-next/src/telemetry/outcome-aggregate.test.ts - no issues
  • services/cloud-agent-next/src/telemetry/outcome-aggregate.ts - 1 issue
  • services/cloud-agent-next/src/telemetry/report-store.ts - no issues
  • services/cloud-agent-next/test/postgres/outcome-aggregate.postgres.test.ts - no issues
  • services/cloud-agent-next/test/postgres/setup.ts - no issues
  • services/cloud-agent-next/vitest.postgres.config.ts - no issues
  • services/cloud-agent-next/wrangler.jsonc - no issues
Verification Notes (click to expand)
  • Retention boundary matches the sweep: retentionCutoff plus gt(created_at, cutoff) in the collector is exactly complementary to lte(created_at, cutoff) in report-store.ts:442, with no off-by-one.
  • Claimed query coverage holds: IDX_cloud_agent_session_runs_terminal_at, IDX_cloud_agent_session_runs_status_terminal, and IDX_cloud_agent_sessions_failure_at exist in packages/db/src/schema.ts.
  • No memory leak introduced: getPgDb(env) creates one max: 1 pool per invocation and is used once per tick, matching the existing per-invocation pattern in telemetry/report-consumer.ts and telemetry/session-reports.ts; the new Postgres suite ends both the writer pool and the reader client in afterAll, and the parent delete cascades to runs.
  • Cron dispatch has no implicit fallback and both branches return; */3 * * * * / 17 2 * * * are asserted against both wrangler.jsonc trigger arrays by src/server.test.ts.
  • Not verified locally: pnpm typecheck/lint/format and the test:postgres suite could not be executed in this sandbox (no installed dependencies or database).

Fix these issues in Kilo Cloud


Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0

Review guidance: REVIEW.md from base branch main

…r null codes

The outcome aggregate coalesced a null failure_code to 'unknown', which is a
valid failure_stage but is not a member of CloudAgentSessionRunFailureCode.
Consumers keyed on the failure-code domain do not recognize it.

The established sentinel for a null code is 'unclassified', as used by
cloud-agent-queue-report.ts and admin-cloud-agent-next-router.ts. A failed run
with both classification fields null is allowed, so the run-level fallback is
reachable; the session-level fallback is currently unreachable because the
sessions check constraint requires stage and code together with failure_at.

Fixing both occurrences keeps the two coalesce expressions consistent. The unit
fixture defaulted failureCode to 'unknown' too, so it mirrored the bug and could
not catch it; the new PostgreSQL regression test asserts the emitted record and
fails without the source fix.
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.

2 participants