Skip to content

Commit e137453

Browse files
authored
refactor(agent): tidy rtk-savings reader from review feedback
Address qa-swarm + Greptile review on #3146: - Guard the parsed JSON before indexing `summary` so a `"null"`/non-object payload returns null explicitly rather than throwing into the caller's catch. - Thread the resolved `env` through to the `rtk gain` exec, so the binary is run against the same environment it was resolved from. - Simplify the run-gain seam to return stdout directly (drop the unused stderr). - Clarify that the counts are RTK's own token estimates (not bytes), and that the zero-guard is on command count. Generated-By: PostHog Code Task-Id: ea7c6128-03d7-492c-a1c8-734175f03927
1 parent 0113380 commit e137453

3 files changed

Lines changed: 25 additions & 19 deletions

File tree

packages/agent/src/server/agent-server.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3394,6 +3394,7 @@ ${signedCommitInstructions}
33943394
notification: {
33953395
jsonrpc: "2.0",
33963396
method: POSTHOG_NOTIFICATIONS.RTK_SAVINGS,
3397+
// snake_case: these land as PostHog event properties.
33973398
params: {
33983399
task_id: this.config.taskId,
33993400
run_id: this.config.runId,

packages/agent/src/server/rtk-savings.test.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ const GAIN_JSON = JSON.stringify({
1515
});
1616

1717
function gain(stdout: string) {
18-
return vi.fn().mockResolvedValue({ stdout, stderr: "" });
18+
return vi.fn().mockResolvedValue(stdout);
1919
}
2020

2121
describe("resolveRtkSavings", () => {
@@ -26,7 +26,7 @@ describe("resolveRtkSavings", () => {
2626
runGain,
2727
});
2828

29-
expect(runGain).toHaveBeenCalledWith("/bundled/rtk");
29+
expect(runGain).toHaveBeenCalledWith("/bundled/rtk", expect.anything());
3030
expect(savings).toEqual({
3131
totalCommands: 2,
3232
inputTokens: 502691,
@@ -54,6 +54,8 @@ describe("resolveRtkSavings", () => {
5454
],
5555
["malformed JSON", "not json"],
5656
["missing summary block", JSON.stringify({ daily: [] })],
57+
["the payload is JSON null", "null"],
58+
["the payload is a non-object", "42"],
5759
])("returns null when %s", async (_label, stdout) => {
5860
const savings = await resolveRtkSavings({
5961
resolveBinary: () => "/bundled/rtk",

packages/agent/src/server/rtk-savings.ts

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@ import { resolveRtkPrefix } from "../adapters/claude/session/rtk";
55
const execFileAsync = promisify(execFile);
66

77
/**
8-
* The `summary` block of `rtk gain --format json` — RTK's own tally of how much
9-
* command output it compressed away before it reached the model.
8+
* The `summary` block of `rtk gain --format json` — RTK's own tally of the
9+
* output it compressed away before it reached the model. The counts are RTK's
10+
* token estimates (its own output labels them "Input/Output tokens" and "Tokens
11+
* saved"), not raw bytes.
1012
*/
1113
export interface RtkSavingsSummary {
1214
totalCommands: number;
@@ -16,26 +18,24 @@ export interface RtkSavingsSummary {
1618
avgSavingsPct: number;
1719
}
1820

19-
interface GainOutput {
20-
stdout: string;
21-
stderr: string;
22-
}
23-
2421
interface ResolveRtkSavingsOptions {
2522
env?: NodeJS.ProcessEnv;
2623
/** Resolves the rtk binary to invoke; undefined disables reporting. Overridable for tests. */
2724
resolveBinary?: (env: NodeJS.ProcessEnv) => string | undefined;
28-
/** Runs `rtk gain` and returns its stdio; overridable for tests. */
29-
runGain?: (binary: string) => Promise<GainOutput>;
25+
/** Runs `rtk gain` and returns its stdout; overridable for tests. */
26+
runGain?: (binary: string, env: NodeJS.ProcessEnv) => Promise<string>;
3027
}
3128

3229
function toFiniteNumber(value: unknown): number {
3330
return typeof value === "number" && Number.isFinite(value) ? value : 0;
3431
}
3532

3633
function parseGainSummary(stdout: string): RtkSavingsSummary | null {
37-
const parsed = JSON.parse(stdout) as { summary?: Record<string, unknown> };
38-
const summary = parsed.summary;
34+
const parsed: unknown = JSON.parse(stdout);
35+
// `JSON.parse("null")` returns null, and a non-object payload has no summary —
36+
// guard before indexing rather than leaning on the caller's try/catch.
37+
if (!parsed || typeof parsed !== "object") return null;
38+
const summary = (parsed as { summary?: Record<string, unknown> }).summary;
3939
if (!summary || typeof summary !== "object") return null;
4040
return {
4141
totalCommands: toFiniteNumber(summary.total_commands),
@@ -46,13 +46,16 @@ function parseGainSummary(stdout: string): RtkSavingsSummary | null {
4646
};
4747
}
4848

49-
async function defaultRunGain(binary: string): Promise<GainOutput> {
50-
const { stdout, stderr } = await execFileAsync(
49+
async function defaultRunGain(
50+
binary: string,
51+
env: NodeJS.ProcessEnv,
52+
): Promise<string> {
53+
const { stdout } = await execFileAsync(
5154
binary,
5255
["gain", "--format", "json"],
53-
{ timeout: 5_000 },
56+
{ timeout: 5_000, env },
5457
);
55-
return { stdout, stderr };
58+
return stdout;
5659
}
5760

5861
/**
@@ -73,9 +76,9 @@ export async function resolveRtkSavings({
7376
if (!binary) return null;
7477

7578
try {
76-
const { stdout } = await runGain(binary);
79+
const stdout = await runGain(binary, env);
7780
const summary = parseGainSummary(stdout);
78-
// Nothing was compressed this runno point emitting a zero.
81+
// No rtk-wrapped commands ran this sessionnothing worth reporting.
7982
if (!summary || summary.totalCommands <= 0) return null;
8083
return summary;
8184
} catch {

0 commit comments

Comments
 (0)