Skip to content

feat(cloud-agent): emit flat agent and code-review health events - #6906

Merged
eshurakov merged 10 commits into
mainfrom
eshurakov/zealous-delta
Sep 29, 2026
Merged

eshurakov merged 10 commits into
mainfrom
eshurakov/zealous-delta

Conversation

@eshurakov

@eshurakov eshurakov commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Bucket Cloud Agent health origin from the existing cli_sessions_v2.created_on_platform (no new column) and add review aggregate indexes with migration 0266.
  • Emit flat agent execution, failure, setup-failure, and open-stock health rows; add review outcome, reason, start, and open-stock rows.
  • Align both collectors to five-minute buckets and crons with a provisional two-minute reporting allowance. Worker uses scheduled slot time for its window; web uses wall clock.

Verification

  • Worker unit suite: 259 files, 7,937 passed, 3 skipped; Worker Postgres: 28 passed; duplication guard passed.
  • Web telemetry and cron-route Jest: 26 passed; web and Worker TypeScript checks passed; changed-file lint/format checks passed; web dependency-cycle check found no cycles (29 imports skipped).
  • Fresh-database Drizzle bootstrap and DB package suites passed (55 schema tests). Local migrated databases include valid concurrent indexes.

Rollout notes

  • Origin is derived from caller-supplied created_on_platform, so the code-review share is self-declared rather than server-decided; accepted for now.
  • New events have not been verified in Axiom or enabled as dashboards/monitors. Reporting allowance is provisional until ingestion lag is measured.
  • Cron retries may duplicate a bucket and missed invocations leave a gap; scheduled cadence removes the systematic overlap from the former three-minute schedule, not platform-level duplicates. Open-stock rows are snapshots, not summable flows.
  • Roll out migration before or with the collectors using the normal deployment workflow; this PR does not deploy.

Comment thread services/cloud-agent-next/src/telemetry/outcome-aggregate.test.ts Outdated
Comment thread services/cloud-agent-next/src/telemetry/outcome-aggregate.test.ts Outdated
Comment thread services/cloud-agent-next/src/telemetry/outcome-aggregate.test.ts Outdated
Comment thread services/cloud-agent-next/src/telemetry/open-stock.test.ts Outdated
Comment thread services/cloud-agent-next/src/telemetry/report-store.test.ts
Comment thread apps/web/src/lib/code-reviews/telemetry/review-health-aggregate.test.ts Outdated
Comment thread apps/web/src/lib/code-reviews/telemetry/review-health-aggregate.ts Outdated
Comment thread apps/web/src/lib/code-reviews/telemetry/review-health-aggregate.ts Outdated
@kilo-code-bot

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

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The three commits since 0508e9069 (drop product_origin, bucket origin from cli_sessions_v2.created_on_platform, add origin Postgres coverage) form a consistent, test-backed refactor: every originBucketExpression call site gained the required left join, no dangling product_origin reference remains, and the regenerated 0266 migration stays index-only.

Files Reviewed (17 files)
  • packages/db/src/migrations/0266_eminent_molecule_man.sql (renamed from 0266_salty_earthquake.sql) - index-only, regenerated
  • packages/db/src/migrations/meta/0266_snapshot.json - generated
  • packages/db/src/migrations/meta/_journal.json - generated
  • packages/db/src/schema.ts - product_origin column and check constraint removed
  • services/cloud-agent-next/src/session-prepare.test.ts
  • services/cloud-agent-next/src/session/session-registration.ts
  • services/cloud-agent-next/src/telemetry/open-stock.ts
  • services/cloud-agent-next/src/telemetry/open-stock.test.ts
  • services/cloud-agent-next/src/telemetry/outcome-aggregate.ts
  • services/cloud-agent-next/src/telemetry/outcome-aggregate.test.ts
  • services/cloud-agent-next/src/telemetry/product-origin.ts - deleted
  • services/cloud-agent-next/src/telemetry/report-store.ts
  • services/cloud-agent-next/src/telemetry/report-store.test.ts
  • services/cloud-agent-next/src/telemetry/session-reports.ts
  • services/cloud-agent-next/src/telemetry/session-reports.test.ts
  • services/cloud-agent-next/test/postgres/open-stock.postgres.test.ts
  • services/cloud-agent-next/test/postgres/outcome-aggregate.postgres.test.ts

Verification notes: origin is now self-declared via the caller-supplied created_on_platform on every path (not just prepareSession), which the rollout notes already accept; cli_sessions_v2.cloud_agent_session_id is uniquely indexed, so the added left joins cannot fan out aggregate counts; the 0266 migration is unshipped and regenerated, so its rename and the column removal are consistent; no memory-leak or secret-logging concerns in the changed code.

Previous Review Summaries (2 snapshots, latest commit 0508e90)

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

Previous review (commit 0508e90)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Incremental re-review of the six files changed since be3292d45c0becafc2ca7cb198553dd94ff8c520 confirms all nine prior findings are resolved: the headline-separation, SQL-shape, and mock-call assertions are now falsifiable, and oldestPendingAgeMs is computed from now() in SQL rather than mixing the JS clock.

Files Reviewed (6 files)
  • apps/web/src/app/api/cron/code-review-outcome-aggregate/route.test.ts
  • apps/web/src/lib/code-reviews/telemetry/review-health-aggregate.ts
  • apps/web/src/lib/code-reviews/telemetry/review-health-aggregate.test.ts
  • services/cloud-agent-next/src/telemetry/open-stock.test.ts
  • services/cloud-agent-next/src/telemetry/outcome-aggregate.test.ts
  • services/cloud-agent-next/src/telemetry/report-store.test.ts

Previous review (commit be3292d)

Status: 9 Issues Found | Recommendation: Address before merge

Executive Summary

The worker/web telemetry rewrite and the product_origin migration are functionally sound, but several new tests contain assertions that can never fail, leaving key new behavior (setup-failure separation, origin persistence) unverified.

Overview

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

WARNING

File Line Issue
services/cloud-agent-next/src/telemetry/outcome-aggregate.test.ts 596 Headline separation assertion is vacuous because runCounts is empty; the test never proves setup failures stay out of execution headlines

SUGGESTION

File Line Issue
services/cloud-agent-next/src/telemetry/outcome-aggregate.test.ts 629 Tautological assertion on a locally declared literal
services/cloud-agent-next/src/telemetry/outcome-aggregate.test.ts 382 Unrelated, unfailable assertion duplicating the empty-input test
services/cloud-agent-next/src/telemetry/open-stock.test.ts 222 not.toContain on camelCase JS names can never fail
services/cloud-agent-next/src/telemetry/report-store.test.ts 234 insert?.conflictValues also passes when no insert happened
apps/web/src/app/api/cron/code-review-outcome-aggregate/route.test.ts 72 401 test does not assert the collectors were not called
apps/web/src/lib/code-reviews/telemetry/review-health-aggregate.test.ts 192 toContain('cloud_agent_code_reviews') always true
apps/web/src/lib/code-reviews/telemetry/review-health-aggregate.ts 101 invalidWaitCount computed but discarded
apps/web/src/lib/code-reviews/telemetry/review-health-aggregate.ts 333 oldestPendingAgeMs mixes JS and DB clocks
Files Reviewed (26 files)
  • apps/web/src/app/api/cron/code-review-outcome-aggregate/route.ts - no issues
  • apps/web/src/app/api/cron/code-review-outcome-aggregate/route.test.ts - 1 issue
  • apps/web/src/lib/code-reviews/telemetry/review-health-aggregate.ts - 2 issues
  • apps/web/src/lib/code-reviews/telemetry/review-health-aggregate.test.ts - 1 issue
  • apps/web/src/lib/code-reviews/telemetry/review-health-aggregate.integration.test.ts - no issues
  • apps/web/vercel.json - no issues
  • packages/db/src/migrations/0266_salty_earthquake.sql - no issues
  • packages/db/src/migrations/meta/0266_snapshot.json - generated, not reviewed
  • packages/db/src/migrations/meta/_journal.json - no issues
  • packages/db/src/schema.ts - no issues
  • services/cloud-agent-next/src/server.ts - no issues
  • services/cloud-agent-next/src/server.test.ts - no issues
  • services/cloud-agent-next/src/session-prepare.test.ts - no issues
  • services/cloud-agent-next/src/session/session-registration.ts - no issues
  • services/cloud-agent-next/src/telemetry/open-stock.ts - no issues
  • services/cloud-agent-next/src/telemetry/open-stock.test.ts - 1 issue
  • services/cloud-agent-next/src/telemetry/outcome-aggregate.ts - no issues
  • services/cloud-agent-next/src/telemetry/outcome-aggregate.test.ts - 3 issues
  • services/cloud-agent-next/src/telemetry/product-origin.ts - no issues
  • services/cloud-agent-next/src/telemetry/report-store.ts - no issues
  • services/cloud-agent-next/src/telemetry/report-store.test.ts - 1 issue
  • services/cloud-agent-next/src/telemetry/session-reports.ts - no issues
  • services/cloud-agent-next/src/telemetry/session-reports.test.ts - no issues
  • services/cloud-agent-next/test/postgres/open-stock.postgres.test.ts - no issues
  • services/cloud-agent-next/test/postgres/outcome-aggregate.postgres.test.ts - no issues
  • services/cloud-agent-next/wrangler.jsonc - no issues

Verification notes: window math (floor((t - 2min)/5min)), group-by ordinals, origin/responsibility bucketing, Drizzle parameterization, await usage, and the schema/migration/index-predicate alignment all check out. The reworked tests do not cover sessions/age-missing counters that were removed, which is consistent with the production change. No memory-leak or secret-logging issues found.

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

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