Skip to content

feat(cli): migrate usage collection to agentsview (CLI 1.0)#120

Closed
ohong wants to merge 7 commits into
mainfrom
oh-agentsview-migration
Closed

feat(cli): migrate usage collection to agentsview (CLI 1.0)#120
ohong wants to merge 7 commits into
mainfrom
oh-agentsview-migration

Conversation

@ohong

@ohong ohong commented May 7, 2026

Copy link
Copy Markdown
Owner

Summary

  • Makes agentsview the default and only routed local collector for straude push.
  • Requires agentsview v0.28.0+, probes the version with a bounded metadata timeout, and runs one unfiltered agentsview usage daily --json --breakdown --offline call for the requested date range.
  • Submits unified collector provenance with collector.unified = "agentsview-v1"; the CLI no longer routes to ccusage, native Codex collection, collector selection env vars, Codex repair flags, or collector merge logic.
  • Keeps the existing ccusage and native Codex modules/tests in the repo as dormant revert code for now.
  • Updates /api/usage/submit trust semantics so CLI-authenticated unified agentsview rows can replace same-device totals, while web-auth spoofing remains blocked and older collector metadata stays valid for already-published clients.
  • Refreshes CLI/privacy/API/planning docs for the agentsview-only migration and the pinned minimum version.
  • Fixes CodeRabbit review gaps: PATH probing ignores empty PATH entries, manual web import cannot get stuck loading on fetch/JSON failures, and API regression mocks now actually exercise trusted-write guard paths.

Why

The goal of this migration is separation of concerns: agentsview owns local session discovery, token accounting, model mapping, and estimated spend for all coding agents it supports. Straude owns auth, upload, feed/profile behavior, and display of already-normalized daily totals.

The previous hybrid direction kept too much accounting logic in Straude. This branch now treats agentsview as the collector boundary and leaves the old collectors only as temporary revert code.

Test plan

  • bun run test in packages/cli
  • bun run build in packages/cli
  • bun run test __tests__/agentsview.test.ts __tests__/commands/push.test.ts __tests__/flows/cli-sync-flow.test.ts in packages/cli
  • bun run test __tests__/api/usage-submit.test.ts in apps/web
  • bun run typecheck in apps/web
  • bun run lint in apps/web
  • git diff --check

Summary by CodeRabbit

  • New Features

    • CLI now uses AgentsView (pinned >= v0.28.0) as the unified local collector; CLI bumped to v1.0.0 and backfill window extended to 30 days.
    • Web import accepts both legacy and AgentsView daily/totals JSON shapes.
  • Bug Fixes

    • Server-side collector metadata validation hardened to prevent spoofed/trusted overwrite paths.
  • Documentation

    • Updated CLI, privacy, import, and landing copy to reflect AgentsView and preserved privacy guarantees.
  • Tests

    • Added/updated tests around AgentsView integration and submit validation.

ohong and others added 5 commits May 7, 2026 14:57
Introduces a new collector adapter for agentsview >= 0.26.1 that mirrors
the ccusage v18 contract. The adapter probes the binary on PATH without
spawning a subprocess, version-gates by parsing `agentsview version`
(tolerates pre-release/build suffixes), and invokes `agentsview usage
daily --json --breakdown --agent claude --offline --since/--until/--timezone`
for the requested window.

Reuses the shared parseDailyUsageOutput so the agentsview output path
benefits from the same token-normalization and bounds checks as ccusage.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds STRAUDE_COLLECTOR=auto|agentsview|legacy in straude push.
- auto: prefer agentsview >= 0.26.1 for Claude when available; keep
  Straude's native Codex collector for Codex; fall back to legacy when
  agentsview is missing/outdated, or while the one-time native-Codex
  repair is pending.
- agentsview: require agentsview >= 0.26.1; still keep native Codex.
- legacy: ccusage + native Codex.

A 3s probe gates the agentsview path so a stuck binary cannot make the
command feel hung. Bumps packages/cli to 1.0.0.

Mirrors the legacy "no Claude data" carve-out for the agentsview branch:
agentsview errors that match a conservative pattern downgrade to
Codex-only sync; genuine failures still exit fatally.

Adds collector_mode/collector_preference/agentsview_available/
codex_repair_pending/agentsview_version to the usage_pushed PostHog
event so we can dashboard rollout shape and detect parity drift.
Telemetry is wrapped in try/catch.

`straude push --debug` now also prints agentsview availability+version,
the codex-native repair flag and timestamp, the IANA timezone passed to
agentsview, and the offline-pricing mode.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
/api/usage/submit now allow-lists collector metadata keys (claude/codex/
unified), enforces a 512-byte cap, and accepts the new "agentsview-v1"
literal for claude and unified provenance. Unknown keys/values return
400 instead of being silently persisted.

Tightens the trusted-Codex-correction rule so that
straude-codex-native-v1 can only lower an existing higher-cost device
row when the request authenticates through the CLI token path. A
Supabase-web session presenting the same collector tag is now treated as
untrusted and the mayOverwriteDevice guard preserves the existing total.

Adds two regression tests:
- rejects unknown collector metadata keys with 400
- rejects web-auth Codex repair attempts even with the trusted collector
  tag (asserts the device upsert + delete chains are not invoked and the
  existing higher cost is preserved)
- accepts agentsview-v1 collector metadata via CLI auth and persists it
  on the device_usage row.

UsageCollectorMeta gains an optional `unified: "agentsview-v1"` field
for forward compatibility with a future fully-consolidated collector.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Updates the user-facing copy that anchored exclusively on ccusage so
existing users aren't surprised by the CLI 1.0 collector model:
- PrivacyPledge component links both ccusage and agentsview as
  legitimate local collectors.
- /privacy "What Straude cannot access" mentions both.
- /cli Data Sources section honestly describes auto's preference order
  (agentsview >= 0.26.1 for Claude when installed; ccusage fallback;
  Codex always on Straude's native collector).
- README narrative aligns with the new collector model.

The privacy pledge itself is unchanged: only aggregated daily totals
ever leave the user's machine.

Manual web import now accepts both schema shapes: the legacy
{ "type": "daily", "data": [...] } and the modern { daily, totals }
that current ccusage v18 and agentsview emit. Adds a toCanonicalEntry
helper that maps either shape to the canonical entry the API expects,
deriving totalTokens from input+output+cacheCreation+cacheRead when
absent. Updates instructions to point users at either
`ccusage daily --json --since YYYYMMDD --until YYYYMMDD` or
`agentsview usage daily --json --since YYYY-MM-DD --until YYYY-MM-DD`.

Does not change the verified-vs-unverified semantics or the
hardcoded web-import device_id constant.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds docs/agentsview-cli-1.0-migration-plan.md with the implementation
plan that landed: hybrid collector strategy (agentsview for Claude,
native for Codex), collector-selection table, accuracy rules, server
metadata contract, test coverage requirements, and the explicit gate
list for a future codex-native retirement PR.

Adds a 2026-05-01 entry in DECISIONS.md recording why CLI 1.0 keeps
codex-native rather than full-cutover, alternatives considered, and the
server rule (only CLI-authenticated straude-codex-native-v1 can lower
totals; agentsview-v1 is provenance only).

Updates docs/agentsview-migration-exploration.md with a 2026-05-01
header pointing at the implementation plan, so the older exploration
isn't read as the active plan.

Updates CHANGELOG (Added/Changed/Fixed) for the agentsview adapter,
codex-only fallback, collector telemetry, debug provenance, dual-shape
import parser, version-regex widening, the server tightening regression
test, and the privacy/landing copy refresh.

Updates docs/CLI.md for STRAUDE_COLLECTOR modes, the new agentsview
invocation, and a troubleshooting note for the version gate. Updates
docs/API.md to reflect the collector metadata field, the 30-day
backfill window, and the device_id requirement.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented May 7, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
straude Ready Ready Preview, Comment May 8, 2026 6:05pm

Request Review

@coderabbitai

coderabbitai Bot commented May 7, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Replaces legacy local collectors with an agentsview-first CLI: adds agentsview subprocess support and PATH detection, generalizes daily-usage parsing for multiple sources, validates collector metadata server-side, normalizes web imports, refactors push to submit unified collector payloads, updates tests, and refreshes docs and package metadata.

Changes

CLI 1.0 Agentsview Collector Integration

Layer / File(s) Summary
Data / Types
apps/web/types/index.ts, docs/API.md
UsageCollectorMeta expanded with unified?: "agentsview-v1" | string; API docs require device_id, document allow-listed collector values, and extend backfill window to 30 days.
Server Validation & API
apps/web/app/api/usage/submit/route.ts
Added validateCollectorMeta() early in POST handler; rejects invalid collector metadata; trust logic now recognizes trusted unified agentsview corrections for overwrite/drop decisions.
Binary PATH Utility & Parsing
packages/cli/src/lib/binary.ts, packages/cli/src/lib/ccusage.ts
Added isBinaryOnPath() used to detect binaries via PATH; ccusage refactored to parseDailyUsageOutput(raw, source) supporting ccusage, agentsview, codex; totalTokens optional for v18 shapes; anomalies include source.
Agentsview Library
packages/cli/src/lib/agentsview.ts
New module: executable resolution, version parsing/gating (MIN_AGENTSVIEW_VERSION = 0.28.0), subprocess exec wrapper, runAgentsViewRawAsync() for usage daily --json --breakdown --offline, and parse delegation.
Push Command Integration
packages/cli/src/commands/push.ts
Refactor: probe/require agentsview, compute ISO since/until, run agentsview, parse entries, filter by server backfill window (max 30 days), compute SHA-256 from agentsview raw, and submit { collector: { unified: AGENTSVIEW_COLLECTOR }, entries }. Removed codex-repair branch and mergeEntries export.
Web Import & Manual UI
apps/web/app/(app)/settings/import/page.tsx, apps/web/app/(app)/post/new/page.tsx
Added CanonicalEntry + toCanonicalEntry() to normalize legacy and agentsview JSON shapes; handleSubmit updated to accept both shapes; UI copy/placeholder updated to mention ccusage or agentsview outputs.
Landing & Privacy Copy
apps/web/app/(landing)/cli/page.tsx, apps/web/app/(landing)/privacy/page.tsx, apps/web/components/landing/PrivacyPledge.tsx, README.md
Updated landing, privacy, privacy-pledge, and README copy to credit agentsview and describe the agentsview-based collection pipeline and 30-day backfill doc changes.
Tests: agentsview & push
packages/cli/__tests__/*
Added agentsview parsing/version/exec tests; updated push/flow tests to mock agentsview, assert version gating, missing-binary errors, date-range forwarding, unified submit payloads, dry-run behavior, telemetry, and config persistence.
Tests: Server
apps/web/__tests__/api/usage-submit.test.ts
Extended tests to reject unknown collector keys, accept agentsview unified metadata and verify stored collector_meta, strengthen lower-value guard tests, and ensure web-auth cannot perform device overwrite/delete repairs.
Docs & Package
docs/*, packages/cli/package.json, packages/cli/README.md, packages/cli/src/index.ts
Pinned agentsview requirement (>= 0.28.0) in docs, extended backfill to 30 days, rewrote Data Sources to agentsview-only, added migration decision/plan, updated changelog/roadmap/privacy, bumped CLI package to 1.0.0, and updated HELP/description.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • ohong/straude#92: Touches the same /api/usage/submit validation surface and collector-trust/overwrite logic.
  • ohong/straude#119: Overlaps on server overwrite/repair guard and Codex trust handling.
  • ohong/straude#15: Related to ccusage parsing/normalization changes used by the new multi-source parser.

Poem

🐰 I found a bin named agentsview, version pinned and spry,

I hop, I parse, I push the days — raw logs stay shy,
Collector tags tucked safe, entries normalized on land,
Docs, tests, and README gleam — Straude’s pipeline in my hand.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 18.37% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: migrating CLI usage collection from ccusage/native Codex to agentsview, aligned with the CLI 1.0 release.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch oh-agentsview-migration
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch oh-agentsview-migration

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/web/app/(app)/settings/import/page.tsx (1)

128-151: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard the submit request with try/catch/finally to avoid sticky loading state.

If fetch rejects (or res.json() throws), setLoading(false) is skipped and the form can remain stuck in “Importing...”. Wrap this block in try/catch/finally and set loading in finally.

Suggested patch
-    const res = await fetch("/api/usage/submit", {
-      method: "POST",
-      headers: { "Content-Type": "application/json" },
-      body: JSON.stringify({
-        entries,
-        source: "web",
-        device_id: "00000000-0000-0000-0000-000000000001",
-        device_name: "web-import",
-      }),
-    });
-
-    if (!res.ok) {
-      const body = await res.json();
-      setError(body.error ?? "Import failed");
-    } else {
-      const body = await res.json();
-      setResults(body.results);
-      posthog.capture("usage_imported", {
-        days_imported: (body.results as ImportResult[]).length,
-        source: "web",
-      });
-    }
-    setLoading(false);
+    try {
+      const res = await fetch("/api/usage/submit", {
+        method: "POST",
+        headers: { "Content-Type": "application/json" },
+        body: JSON.stringify({
+          entries,
+          source: "web",
+          device_id: "00000000-0000-0000-0000-000000000001",
+          device_name: "web-import",
+        }),
+      });
+
+      const body = await res.json();
+      if (!res.ok) {
+        setError(body.error ?? "Import failed");
+      } else {
+        setResults(body.results);
+        posthog.capture("usage_imported", {
+          days_imported: (body.results as ImportResult[]).length,
+          source: "web",
+        });
+      }
+    } catch {
+      setError("Import failed. Please try again.");
+    } finally {
+      setLoading(false);
+    }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/app/`(app)/settings/import/page.tsx around lines 128 - 151, The
submit flow currently calls fetch and res.json() without error handling so
setLoading(false) can be skipped; wrap the network/response handling in a
try/catch/finally inside the same async function (the block surrounding
fetch("/api/usage/submit") that uses setError, setResults and posthog.capture)
so any thrown error is caught and setError(...) is called in catch, and
setLoading(false) is always executed in finally; keep the existing logic for
handling res.ok and posthog.capture("usage_imported") but move res.json() calls
into the try block and ensure you only await res.json() once per branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/web/__tests__/api/usage-submit.test.ts`:
- Around line 979-987: The negative assertions are unreachable because
deviceUpsertChain and deviceDeleteChain are never returned by the fromFn mock;
update the fromFn mock (the mock that returns query chains) to explicitly return
deviceUpsertChain and deviceDeleteChain for the calls under test (e.g., use
fromFn.mockImplementation or mockReturnValueOnce to return deviceUpsertChain for
the upsert flow and deviceDeleteChain for the delete flow), so that
not.toHaveBeenCalled() checks are meaningful; reference the deviceUpsertChain,
deviceDeleteChain and fromFn mocks when making the change.

In `@docs/agentsview-migration-exploration.md`:
- Around line 11-23: Update the document's top-level metadata to reflect this
concrete CLI 1.0 implementation decision: change the Status field from an
"Exploration" or draft value to something like "Implementation" (or
"Adopted/Committed") and set Last updated to 2026-05-01 (the date in the section
heading); also update any Summary/Title lines that still read as exploratory to
reference the CLI 1.0 implementation plan and link to
docs/agentsview-cli-1.0-migration-plan.md for details so readers won't get mixed
signals about whether this is exploratory or finalized.

In `@packages/cli/README.md`:
- Around line 29-31: Update the README.md smart-sync description to consistently
state the 30-day cap: change the bullet that currently reads “Subsequent runs:
pushes all days since the last sync (up to 7 days)” to mention the 30-day
maximum instead (e.g., “up to 30 days”), so the three bullets (“First run: ...”,
“Subsequent runs: ...”, “Already synced today: ...”) and the later documentation
lines (around the existing 51-53 text) all reflect the same 30-day cap.

In `@packages/cli/src/commands/push.ts`:
- Around line 379-385: The metadata-only agentsView version probe is using the
full options.timeoutMs (via getAgentsViewVersion) which can block if users pass
a large timeout; change the call to getAgentsViewVersion so it uses the same
capped/short probe timeout that isSupportedAgentsViewInstalled uses (i.e., pass
the capped probe timeout variable/value used earlier rather than
options.timeoutMs) so the eager, non-gating version fetch remains best-effort
and bounded.
- Around line 78-81: The isMissingAgentsViewClaudeDataError function's regex is
too strict and misses messages like "No valid Claude data directories found";
update isMissingAgentsViewClaudeDataError to broaden its matching (either reuse
the same pattern as isMissingClaudeDataError or expand the current regex to
allow optional words like "valid" and arbitrary tokens between "no" and "claude"
and to match "data directories found"/"directories" variants,
case-insensitively) so that missing-Claude-data errors trigger the graceful
Codex-only fallback instead of a fatal exit.

In `@packages/cli/src/lib/binary.ts`:
- Around line 6-10: The PATH probing currently uses dirs = (process.env.PATH ||
"").split(delimiter) and will treat empty segments as the current directory,
causing false positives; update the dirs construction to filter out empty
entries (e.g., .filter(Boolean) or .filter(dir => dir !== "")) before running
suffixes.some and existsSync(join(dir, binary + ext)). Keep references to dirs,
suffixes, existsSync, join, binary and delimiter so the change is localized to
the existing probe logic.

---

Outside diff comments:
In `@apps/web/app/`(app)/settings/import/page.tsx:
- Around line 128-151: The submit flow currently calls fetch and res.json()
without error handling so setLoading(false) can be skipped; wrap the
network/response handling in a try/catch/finally inside the same async function
(the block surrounding fetch("/api/usage/submit") that uses setError, setResults
and posthog.capture) so any thrown error is caught and setError(...) is called
in catch, and setLoading(false) is always executed in finally; keep the existing
logic for handling res.ok and posthog.capture("usage_imported") but move
res.json() calls into the try block and ensure you only await res.json() once
per branch.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b5381d9a-bc8d-4097-a5c6-50f42aa37813

📥 Commits

Reviewing files that changed from the base of the PR and between f78af30 and 06d20e4.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (24)
  • README.md
  • apps/web/__tests__/api/usage-submit.test.ts
  • apps/web/app/(app)/post/new/page.tsx
  • apps/web/app/(app)/settings/import/page.tsx
  • apps/web/app/(landing)/cli/page.tsx
  • apps/web/app/(landing)/privacy/page.tsx
  • apps/web/app/api/usage/submit/route.ts
  • apps/web/components/landing/PrivacyPledge.tsx
  • apps/web/types/index.ts
  • docs/API.md
  • docs/CHANGELOG.md
  • docs/CLI.md
  • docs/DECISIONS.md
  • docs/agentsview-cli-1.0-migration-plan.md
  • docs/agentsview-migration-exploration.md
  • packages/cli/README.md
  • packages/cli/__tests__/agentsview.test.ts
  • packages/cli/__tests__/commands/push.test.ts
  • packages/cli/package.json
  • packages/cli/src/commands/push.ts
  • packages/cli/src/index.ts
  • packages/cli/src/lib/agentsview.ts
  • packages/cli/src/lib/binary.ts
  • packages/cli/src/lib/ccusage.ts

Comment thread apps/web/__tests__/api/usage-submit.test.ts
Comment thread docs/agentsview-migration-exploration.md Outdated
Comment thread packages/cli/README.md Outdated
Comment thread packages/cli/src/commands/push.ts Outdated
Comment thread packages/cli/src/commands/push.ts Outdated
Comment thread packages/cli/src/lib/binary.ts Outdated
@ohong ohong changed the title feat(cli): migrate Claude collection to agentsview (CLI 1.0) feat(cli): migrate usage collection to agentsview (CLI 1.0) May 8, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
docs/agentsview-cli-1.0-migration-plan.md (1)

132-139: 💤 Low value

Consider restructuring the non-goals list for better readability.

The explicit non-goals section is valuable, but four consecutive sentences beginning with "No" (lines 135-138) slightly reduce readability. Consider restructuring to improve flow while maintaining clarity.

♻️ Proposed refactor for improved readability
 ## Explicit Non-Goals
 
-- No Straude-maintained pricing table for agent usage.
-- No fallback to ccusage.
-- No fallback to native Codex parsing.
-- No per-agent collection logic in Straude.
-- No server-side recomputation of local collector costs.
+This migration explicitly excludes:
+
+- Straude-maintained pricing tables for agent usage
+- Fallback to ccusage or native Codex parsing
+- Per-agent collection logic in Straude
+- Server-side recomputation of local collector costs
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/agentsview-cli-1.0-migration-plan.md` around lines 132 - 139, Reword the
"Explicit Non-Goals" section to avoid four consecutive list items starting with
"No" by converting each item into positive noun-phrase bullets or short clauses
(e.g., "Straude-maintained pricing table for agent usage is out of scope" →
"Straude-maintained pricing table for agent usage (out of scope)"), or group
related items into a sublist, keeping the header "Explicit Non-Goals" and the
existing items' intents intact; update the four lines that currently begin with
"No" so each reads as a concise out-of-scope statement that improves flow and
readability.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/agentsview-cli-1.0-migration-plan.md`:
- Line 62: Update the compound modifier "post creation behavior" in the sentence
that currently reads Keep the 30-day backfill window, `device_id` requirement,
device aggregation, and post creation behavior unchanged. to use a hyphenated
form: replace "post creation behavior" with "post-creation behavior" so the
modifier is grammatically correct.

In `@docs/agentsview-migration-exploration.md`:
- Around line 6-7: Update the phrasing that presents "use agentsview as Straude
CLI's single local collector for all supported coding agents" to clearly mark it
as the target end-state/next phase rather than current CLI 1.0 behavior;
specifically, in the sentence referencing agentsview and any mentions of
removing `STRAUDE_COLLECTOR` or legacy/native paths (also the nearby block
around lines 91-94), add a short qualifier like "target state" or "next phase"
and a brief note that the PR ships transitional behavior and retains the
legacy/native paths for CLI 1.0 to avoid implying features have already been
removed.

In `@packages/cli/README.md`:
- Line 29: Update the README line that currently says "First run: opens a
browser tab to authenticate, then pushes recent usage." to explicitly state the
backfill window (e.g., "pushes the last 3 days of usage" or "pushes recent usage
(typically 3 days)") so readers know the exact time range; edit the sentence
text in packages/cli/README.md where the "First run" description appears to
replace "recent usage" with the chosen explicit phrasing.

---

Nitpick comments:
In `@docs/agentsview-cli-1.0-migration-plan.md`:
- Around line 132-139: Reword the "Explicit Non-Goals" section to avoid four
consecutive list items starting with "No" by converting each item into positive
noun-phrase bullets or short clauses (e.g., "Straude-maintained pricing table
for agent usage is out of scope" → "Straude-maintained pricing table for agent
usage (out of scope)"), or group related items into a sublist, keeping the
header "Explicit Non-Goals" and the existing items' intents intact; update the
four lines that currently begin with "No" so each reads as a concise
out-of-scope statement that improves flow and readability.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0599533a-a098-41c6-b1f1-78ce14fda087

📥 Commits

Reviewing files that changed from the base of the PR and between 06d20e4 and 94d3e18.

📒 Files selected for processing (8)
  • docs/CHANGELOG.md
  • docs/CLI.md
  • docs/DECISIONS.md
  • docs/agentsview-cli-1.0-migration-plan.md
  • docs/agentsview-migration-exploration.md
  • packages/cli/README.md
  • packages/cli/__tests__/agentsview.test.ts
  • packages/cli/src/lib/agentsview.ts
✅ Files skipped from review due to trivial changes (2)
  • docs/CHANGELOG.md
  • docs/DECISIONS.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/cli/tests/agentsview.test.ts
  • packages/cli/src/lib/agentsview.ts

Comment thread docs/agentsview-cli-1.0-migration-plan.md Outdated
Comment thread docs/agentsview-migration-exploration.md Outdated
Comment thread packages/cli/README.md Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/cli/src/commands/push.ts`:
- Around line 358-384: Filter response.results for successful actions (e.g.,
action === "created" || action === "updated"), map those successful result
indices back to the corresponding entries (response.results is ordered to match
entries), and then compute latestDate, totalCost, totalTokens, days_pushed,
datesCreated, and datesUpdated only from that subset; call
updateLastPushDate(latestDate) only when there is at least one successful entry
and send telemetry (posthog.capture) using the filtered totals and counts (also
compute anomalyCounts only over the successful entries) instead of using totals
derived from every attempted entry.

In `@packages/cli/src/lib/agentsview.ts`:
- Around line 47-67: The callback for execFileCb is currently treating stderr as
a property on the ExecException (error.stderr) which loses subprocess error
output; update the callback signature in the Promise resolver from (err, stdout)
to (err, stdout, stderr) and use the stderr value (e.g. const detail =
stderr?.trim() || error.message || "unknown error") when building the rejection
message, while keeping existing timeout/killed handling for error.killed or
error.signal and retaining execFileCb, cmd, cmdArgs and
DEFAULT_SUBPROCESS_TIMEOUT_MS as-is.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3a1a467a-90b2-415c-afc5-4cf47daf4da7

📥 Commits

Reviewing files that changed from the base of the PR and between 94d3e18 and 46bc863.

📒 Files selected for processing (24)
  • README.md
  • apps/web/__tests__/api/usage-submit.test.ts
  • apps/web/app/(app)/settings/import/page.tsx
  • apps/web/app/(landing)/cli/page.tsx
  • apps/web/app/(landing)/privacy/page.tsx
  • apps/web/app/api/usage/submit/route.ts
  • apps/web/components/landing/PrivacyPledge.tsx
  • apps/web/types/index.ts
  • docs/API.md
  • docs/CHANGELOG.md
  • docs/CLI.md
  • docs/DECISIONS.md
  • docs/ROADMAP.md
  • docs/agentsview-cli-1.0-migration-plan.md
  • docs/agentsview-migration-exploration.md
  • packages/cli/README.md
  • packages/cli/__tests__/agentsview.test.ts
  • packages/cli/__tests__/commands/push.test.ts
  • packages/cli/__tests__/flows/cli-sync-flow.test.ts
  • packages/cli/__tests__/resolve-push-date-range.test.ts
  • packages/cli/src/commands/push.ts
  • packages/cli/src/index.ts
  • packages/cli/src/lib/agentsview.ts
  • packages/cli/src/lib/binary.ts
💤 Files with no reviewable changes (1)
  • packages/cli/tests/resolve-push-date-range.test.ts
✅ Files skipped from review due to trivial changes (8)
  • packages/cli/src/index.ts
  • apps/web/app/(landing)/privacy/page.tsx
  • apps/web/app/(landing)/cli/page.tsx
  • README.md
  • docs/DECISIONS.md
  • docs/ROADMAP.md
  • docs/API.md
  • packages/cli/README.md
🚧 Files skipped from review as they are similar to previous changes (4)
  • packages/cli/src/lib/binary.ts
  • apps/web/types/index.ts
  • packages/cli/tests/agentsview.test.ts
  • apps/web/app/(app)/settings/import/page.tsx

Comment on lines 358 to +384
const latestDate = entries.reduce(
(latest, e) => (e.date > latest ? e.date : latest),
entries[0]!.date,
);
if (shouldRunCodexRepair && !codexCollectFailed && blockedDates.size === 0) {
config.codex_native_repair_completed_at = new Date().toISOString();
config.last_push_date = latestDate;
saveConfig(config);
} else {
updateLastPushDate(latestDate);
}
updateLastPushDate(latestDate);

const totalCost = entries.reduce((sum, e) => sum + e.costUSD, 0);
const totalTokens = entries.reduce((sum, e) => sum + e.totalTokens, 0);
const datesCreated = response.results.filter((r) => r.action === "created").length;
const datesUpdated = response.results.filter((r) => r.action === "updated").length;
posthog.capture({
distinctId: getDistinctId(config),
event: "usage_pushed",
properties: {
days_pushed: entries.length,
dates_created: datesCreated,
dates_updated: datesUpdated,
total_cost_usd: Math.round(totalCost * 100) / 100,
total_tokens: totalTokens,
dry_run: false,
anomalies_medium_low: anomalyCounts.mediumLow,
anomalies_low_confidence: anomalyCounts.low,
anomalies_unresolved: anomalyCounts.unresolved,
},
});
try {
posthog.capture({
distinctId: getDistinctId(config),
event: "usage_pushed",
properties: {
days_pushed: entries.length,
dates_created: datesCreated,
dates_updated: datesUpdated,
total_cost_usd: Math.round(totalCost * 100) / 100,
total_tokens: totalTokens,
dry_run: false,
anomalies_medium_low: anomalyCounts.mediumLow,
anomalies_low_confidence: anomalyCounts.low,
anomalies_unresolved: anomalyCounts.unresolved,
collector_mode: "agentsview-unified",
agentsview_version: agentsViewVersion,
},

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Only advance sync state from successful rows.

/api/usage/submit can now return 207, but this block still derives last_push_date, days_pushed, total_cost_usd, and total_tokens from every attempted entry. If the newest date fails while an older one succeeds, the next smart sync skips the failed day entirely. The telemetry below is overstated for the same reason.

Suggested fix
-  const latestDate = entries.reduce(
+  const successfulDates = new Set(response.results.map((result) => result.date));
+  const successfulEntries = entries.filter((entry) => successfulDates.has(entry.date));
+
+  const latestDate = successfulEntries.reduce(
     (latest, e) => (e.date > latest ? e.date : latest),
-    entries[0]!.date,
+    successfulEntries[0]!.date,
   );
   updateLastPushDate(latestDate);
 
-  const totalCost = entries.reduce((sum, e) => sum + e.costUSD, 0);
-  const totalTokens = entries.reduce((sum, e) => sum + e.totalTokens, 0);
+  const totalCost = successfulEntries.reduce((sum, e) => sum + e.costUSD, 0);
+  const totalTokens = successfulEntries.reduce((sum, e) => sum + e.totalTokens, 0);
   const datesCreated = response.results.filter((r) => r.action === "created").length;
   const datesUpdated = response.results.filter((r) => r.action === "updated").length;
   try {
     posthog.capture({
       distinctId: getDistinctId(config),
       event: "usage_pushed",
       properties: {
-        days_pushed: entries.length,
+        days_pushed: successfulEntries.length,
         dates_created: datesCreated,
         dates_updated: datesUpdated,
         total_cost_usd: Math.round(totalCost * 100) / 100,
         total_tokens: totalTokens,
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/cli/src/commands/push.ts` around lines 358 - 384, Filter
response.results for successful actions (e.g., action === "created" || action
=== "updated"), map those successful result indices back to the corresponding
entries (response.results is ordered to match entries), and then compute
latestDate, totalCost, totalTokens, days_pushed, datesCreated, and datesUpdated
only from that subset; call updateLastPushDate(latestDate) only when there is at
least one successful entry and send telemetry (posthog.capture) using the
filtered totals and counts (also compute anomalyCounts only over the successful
entries) instead of using totals derived from every attempted entry.

Comment on lines +47 to +67
return new Promise((resolve, reject) => {
execFileCb(cmd, cmdArgs, {
encoding: "utf-8",
timeout: timeoutMs ?? DEFAULT_SUBPROCESS_TIMEOUT_MS,
maxBuffer: 10 * 1024 * 1024,
shell: process.platform === "win32",
}, (err, stdout) => {
if (!err) {
resolve(stdout);
return;
}
const error = err as ExecError;
if (error.killed || error.signal === "SIGTERM") {
reject(new Error(
"agentsview timed out. Try running `agentsview usage daily --json --offline` directly to verify it works.",
));
return;
}
const detail = error.stderr?.trim() || error.message || "unknown error";
reject(new Error(`agentsview failed: ${detail}`));
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

What is the callback signature of Node.js child_process.execFile, and is stderrexposed on theExecException object or as the callback's third argument?

💡 Result:

The callback signature of Node.js child_process.execFile is: callback(error, stdout, stderr) Where on success error is null, and stdout/stderr are the child process outputs. [1] stderr is exposed as the callback’s third argument (stderr), not on the ExecException/“error” object itself. The docs describe that stdout and stderr output are provided to the callback as stdout (2nd argument) and stderr (3rd argument). [1][2]

Citations:


🏁 Script executed:

fd -t f agentsview.ts

Repository: ohong/straude

Length of output: 92


🏁 Script executed:

sed -n '47,67p' packages/cli/src/lib/agentsview.ts

Repository: ohong/straude

Length of output: 805


🏁 Script executed:

cat -n packages/cli/src/lib/agentsview.ts | head -80

Repository: ohong/straude

Length of output: 3204


Capture stderr from execFile's third callback argument.

Node.js execFile passes stderr as the callback's third argument, not on the ExecException object. The current code attempts to access error.stderr?.trim(), which is unreliable and causes most non-timeout failures to fall back to error.message, losing the actual subprocess error output. Update the callback signature to accept the stderr parameter and use it instead.

Suggested fix
-    }, (err, stdout) => {
+    }, (err, stdout, stderr) => {
       if (!err) {
         resolve(stdout);
         return;
       }
       const error = err as ExecError;
       if (error.killed || error.signal === "SIGTERM") {
         reject(new Error(
           "agentsview timed out. Try running `agentsview usage daily --json --offline` directly to verify it works.",
         ));
         return;
       }
-      const detail = error.stderr?.trim() || error.message || "unknown error";
+      const detail = stderr?.trim() || error.message || "unknown error";
       reject(new Error(`agentsview failed: ${detail}`));
     });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/cli/src/lib/agentsview.ts` around lines 47 - 67, The callback for
execFileCb is currently treating stderr as a property on the ExecException
(error.stderr) which loses subprocess error output; update the callback
signature in the Promise resolver from (err, stdout) to (err, stdout, stderr)
and use the stderr value (e.g. const detail = stderr?.trim() || error.message ||
"unknown error") when building the rejection message, while keeping existing
timeout/killed handling for error.killed or error.signal and retaining
execFileCb, cmd, cmdArgs and DEFAULT_SUBPROCESS_TIMEOUT_MS as-is.

@ohong

ohong commented Jun 11, 2026

Copy link
Copy Markdown
Owner Author

Pausing this migration work for now, will reopen if it's needed.

@ohong ohong closed this Jun 11, 2026
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