diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 87392432..f825608e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -230,6 +230,20 @@ jobs: run: cd packages/sdk && npx tsc - name: Build docs run: pnpm docs:build + # `workers/host/src/pages.ts` is build output (build.js inlines store/ into it) and is + # gitignored, so a fresh checkout does not have it — and `pnpm test` collects + # workers/host/src/admin-api-proxy.test.ts, which imports the host index.ts, which imports + # ./pages.js at module load. Without these the suite fails to collect that file. + # + # The console build was the LAST step of this job, after the tests that needed it. It stays a + # build-succeeds check by running here; it just also produces the bundle build.js reads, so + # it is one step doing both jobs rather than the same build run twice. + - name: Build Console + run: cd store/console && npx vite build + - name: Build Admin + run: cd store/admin && npx vite build + - name: Build host pages + run: cd workers/host && node build.js - run: pnpm test - name: E2E projects still select the tests they claim to (#740) # `playwright.config.ts` scopes the WebKit project with `grep: /mobile — /` and @@ -287,5 +301,3 @@ jobs: # there passed CI, landed on main, and failed at DEPLOY instead of at PR time. - name: Typecheck Admin run: cd store/admin && npx tsc --noEmit - - name: Build Console - run: cd store/console && npx vite build diff --git a/.github/workflows/deploy-api.yml b/.github/workflows/deploy-api.yml index 8161e9b6..ab09f46b 100644 --- a/.github/workflows/deploy-api.yml +++ b/.github/workflows/deploy-api.yml @@ -45,6 +45,25 @@ jobs: - name: Build SDK run: cd packages/sdk && npx tsc + # `workers/host/src/pages.ts` is BUILD OUTPUT (build.js inlines store/ into it) and is + # gitignored, so it does not exist in a fresh checkout. `pnpm test` includes + # workers/host/src/admin-api-proxy.test.ts, which imports the host worker's index.ts, which + # imports ./pages.js at module load — so without these three steps the suite fails to COLLECT + # that file and the whole deploy stops, for a reason unrelated to the API. build.js reads the + # console and admin Vite bundles, so both have to be built first; this is the same chain, in + # the same order, as deploy-host.yml. + - name: Build console React app + working-directory: store/console + run: npx vite build + + - name: Build admin React app + working-directory: store/admin + run: npx vite build + + - name: Build host pages + working-directory: workers/host + run: node build.js + - name: Run tests run: pnpm test diff --git a/agents/coder/web/src/EnginesModal.tsx b/agents/coder/web/src/EnginesModal.tsx index dac6c8eb..36f1a5c5 100644 --- a/agents/coder/web/src/EnginesModal.tsx +++ b/agents/coder/web/src/EnginesModal.tsx @@ -125,7 +125,7 @@ export default function EnginesModal({ instanceId, engines: initial, defaultEngi {invocation && ( diff --git a/e2e/admin.spec.ts b/e2e/admin.spec.ts index 7a824133..16208542 100644 --- a/e2e/admin.spec.ts +++ b/e2e/admin.spec.ts @@ -29,7 +29,14 @@ import { type Page, expect, test } from "@playwright/test"; * through it would trade the thing being protected for coverage of the thing protecting it. */ -const API = "https://api.proagentstore.online"; +/** + * The admin app talks to the host's SAME-ORIGIN proxy, not the API host directly. So an + * intercepted request's pathname is `/admin/api` + the API path, and the handlers below — + * which match the API path itself — have to strip the prefix. Matching the raw pathname + * silently missed every route and left each page on its signed-out/error branch. + */ +const API = "/admin/api"; +const apiPath = (url: string) => new URL(url).pathname.slice(API.length); const TEST_TOKEN = "test-pags-admin-token"; type Json = Record; @@ -64,8 +71,7 @@ async function mockAdmin(page: Page, instances: Json[] = []): Promise const mock: AdminMock = { requests: [], deleteResponses: [] }; await page.route(`${API}/**`, async (route) => { - const url = new URL(route.request().url()); - const path = url.pathname; + const path = apiPath(route.request().url()); const method = route.request().method(); const json = (data: unknown, status = 200) => route.fulfill({ status, contentType: "application/json", body: JSON.stringify(data) }); @@ -328,7 +334,7 @@ async function mockAdminSurfaces(page: Page) { const bucket = (key: string) => ({ key, label: key, inputTokens: 100, outputTokens: 50, costMicros: 2000, calls: 7 }); await page.route(`${API}/**`, async (route) => { - const path = new URL(route.request().url()).pathname; + const path = apiPath(route.request().url()); const json = (data: unknown) => route.fulfill({ status: 200, contentType: "application/json", body: JSON.stringify(data) }); if (path === "/v1/admin/me") return json({ admin: true }); diff --git a/scripts/check-file-size.mjs b/scripts/check-file-size.mjs index c1e445da..4a52f82d 100644 --- a/scripts/check-file-size.mjs +++ b/scripts/check-file-size.mjs @@ -158,7 +158,12 @@ const PINS = { // `/resume`, `/end`). That is a real split and it is not #738's — a four-field reporting fix is // the wrong commit to move eight routes in, and doing both at once would make the behaviour // change unreviewable against the move. - "workers/api/src/routes/coding.ts": 810, + // +5 at #731 (1e038c43, pin raised after the fact): `/capture` now answers with an + // `engineInvocationReport` built from the session's clientType, its launch command and the mode + // the runner reports — the three readings the console needs to say "running raw" without + // guessing. It is one call and one response field on the attach-and-observe side of the seam + // named above, so it does not move that split; the split is still the next raise's job. + "workers/api/src/routes/coding.ts": 815, // +2 for #496 AC2: the owner-initiated resync-identity route is mounted from a new sub-module // (instances-identity.ts) to keep this file's size honest; the two new lines are the import // and the register call. Raised rather than split: the whole change is a mount and an import. @@ -627,7 +632,14 @@ const PINS = { // immediately BEFORE calling this, and a warm re-attach reports nothing, so the obvious form // blanks a correct banner on every open that has one. That is invisible in the diff and it is // the exact tidy-up a later reader would make. Still #305's landing/session split as the seam. - "agents/coder/web/src/CodingTab.tsx": 1478, + // +12 at #731 (1e038c43, pin raised after the fact): the invocation-mode strip — a second piece + // of state on the session header and the JSX that renders it beside the existing engine badge. + // The badge and the invocation line are deliberately ONE block with a shared `warn` tone rather + // than two independent notices, which is what the extra nesting buys: an engine that is signed + // in but running raw is one situation, and reporting it as two adjacent warnings is how the + // strip #549 already widened would keep widening. The reading itself is not here — it is + // `engine-invocation-mode.ts`, with its own tests. Still #305's landing/session split as the seam. + "agents/coder/web/src/CodingTab.tsx": 1490, // +18 for #425: two Chrome launch flags, the args array reformatted one-per-line to fit them, // and the paragraph saying why they are a PAIR. `--use-fake-ui-for-media-stream` on its own // auto-GRANTS the real microphone to any page the agent drives — strictly worse than the prompt @@ -1020,7 +1032,13 @@ const PINS = { // (this table has no retention cron), why `warn` rather than `error`, and why its message keeps // the 600 characters #517 preserved instead of the 200 the summary row cuts to. // +31 for #739: the operator manual notice block — a D1 query + conditional prompt injection. - "workers/api/src/agent-think.ts": 1238, + // +13 at #732 (d5701824, pin raised after the fact): the chat call site now hands the usage + // ledger a per-section prompt breakdown (system / messages / tools) so `llm.prompt_sections` + // can say WHICH part of a prompt is the expensive one. Twelve of the thirteen lines are the + // literal that splits the body it already had; the estimator and its logging are + // lib/prompt-section-estimates.ts. Raised rather than split: the labels have to be produced + // where the message array is assembled, and moving them out would hand that module the prompt. + "workers/api/src/agent-think.ts": 1251, // +44 at #379, and roughly two thirds of it is prose. A machine's identity stopped being its // hostname: the registration body accepts a stable `machineId` plus the hostnames that machine // has worn, the node upsert stores the id (with the COALESCE that stops an OLDER CLI erasing @@ -1534,7 +1552,12 @@ const PINS = { // +4 at #739: two raised pins (tool-registry, agent-think) + this note + blank line. // +13 at #687: new entry for github-browse.ts (11-line rationale + pin) + this note + blank. // +7 at #688: raise of github-browse.ts pin (6-line rationale + pin bump) + this note + own pin bump. - "scripts/check-file-size.mjs": 1618, + // +23 on the red-main fix-forward: three raises (routes/coding, CodingTab, agent-think) whose + // growth landed on main WITHOUT the pin move this guard asks for, so CI went red at 50aecb9c + // and stayed red for five days while nothing deployed. The reasons are written from the diffs + // after the fact rather than by their authors, which is the cost of raising a pin late and the + // argument for raising it in the same commit. Plus this note. + "scripts/check-file-size.mjs": 1641, }; /** diff --git a/workers/api/src/lib/engine-metering.test.ts b/workers/api/src/lib/engine-metering.test.ts index 2b414a2b..fa27b4f8 100644 --- a/workers/api/src/lib/engine-metering.test.ts +++ b/workers/api/src/lib/engine-metering.test.ts @@ -93,6 +93,32 @@ describe("classifyEngineMetering", () => { expect(classifyEngineMetering("terminal", "").metered).toBe(false); }); + it("does not meter a Codex session that is not launched as `exec --json`", () => { + // Same binary, opposite verdict: `codex exec --json` emits per-turn tokens, `codex chat` + // emits prose. The engine NAME cannot answer this, which is why the launch command is an + // input to the classifier rather than a check performed around it. + const v = classifyEngineMetering("headless", "codex", "codex chat"); + expect(v.metered).toBe(false); + expect(v.reason.length).toBeGreaterThan(20); + expect(classifyEngineMetering("headless", "codex", "codex exec --json").metered).toBe(true); + }); + + it("reads a missing launch command as the default invocation, so old rows do not reclassify", () => { + // The argument arrived after these sessions were written. Treating "not recorded" as raw + // would retroactively turn every measured Codex session into an unmetered one. + expect(classifyEngineMetering("headless", "codex", null).metered).toBe(true); + expect(classifyEngineMetering("headless", "codex", " ").metered).toBe(true); + }); + + it("does not read an unrecognised engine as Claude, whatever the launch command says", () => { + // The regression that made this a guard: routing the engine name through `asClient` first + // collapses anything outside its four-name list to "claude", which then reads as + // structured — so these all silently claimed to be measured and recorded no absence. + for (const engine of ["aider", "opencode", "goose", "amp", "crush", "cursor-agent"]) { + expect(classifyEngineMetering("headless", engine).metered, engine).toBe(false); + } + }); + it("always explains itself in a sentence a page can print", () => { for (const v of [ classifyEngineMetering("headless", "claude"), @@ -274,6 +300,25 @@ describe("noteUnmeteredHeadlessDrive — the other row of the 2x2 (#556)", () => expect(new Set(runs.map((r) => String(r.args[0]))).size).toBe(1); }); + it("records the absence for a Codex session launched raw, not just for a raw engine", async () => { + // The behaviour #556's fix-forward had to preserve: a metered ENGINE driven through an + // unmetered INVOCATION is unmetered, and the row has to say so. + const { runs, env } = fakeDb(); + await noteUnmeteredHeadlessDrive(env, { userId: "u1", instanceId: "i1" }, { id: "csess-9", clientType: "codex", launchCommand: "codex chat" }); + expect(runs).toHaveLength(1); + expect(contextOf(runs[0]).paneCommand).toBe("codex"); + }); + + it("records the absence for an AI CLI outside the four known clientTypes", async () => { + // Regression guard. These read as "claude" through `asClient` and recorded nothing, so a + // day of unmeasured aider work looked identical to a day of measured Claude Code work. + const { runs, env } = fakeDb(); + await noteUnmeteredHeadlessDrive(env, { userId: "u1", instanceId: "i1" }, { id: "csess-9", clientType: "aider" }); + expect(runs).toHaveLength(1); + expect(contextOf(runs[0]).aiCli).toBe(true); + expect(contextOf(runs[0]).target).toBe("aider:csess-9"); + }); + it("says something honest about an engine whose clientType is missing", async () => { // Never "nothing was spent". An unknown engine is unmeasurable until proven otherwise — // the asymmetry `AI_CLI_COMMANDS` is documented to preserve. diff --git a/workers/api/src/lib/engine-metering.ts b/workers/api/src/lib/engine-metering.ts index 168e126f..f86fcbf1 100644 --- a/workers/api/src/lib/engine-metering.ts +++ b/workers/api/src/lib/engine-metering.ts @@ -29,7 +29,7 @@ import { logEvent } from "./events.js"; import type { UnmeteredUsageSummary } from "./usage-shape.js"; import type { Env } from "../types.js"; -import { asClient, expectedEngineInvocationMode } from "./coding-engines.js"; +import { expectedEngineInvocationMode } from "./coding-engines.js"; /** * How the platform is driving the CLI. @@ -105,8 +105,14 @@ export function isAiCli(raw: string | null | undefined): boolean { * * The reason string is the product here. "unmetered: true" is not something a Usage page can * print; a sentence saying which part of the pipeline drops the number is. + * + * `launchCommand` is the third input the verdict genuinely depends on, and only for Codex: it + * reports tokens under `exec --json` and says nothing at all under any other subcommand, so the + * engine NAME alone cannot answer the question for it. Absent (an older session row that never + * recorded one) reads as the default invocation, which is structured — the same answer this + * function gave before the argument existed, so nothing reclassifies retroactively. */ -export function classifyEngineMetering(driver: EngineDriver, engine?: string | null): MeteringVerdict { +export function classifyEngineMetering(driver: EngineDriver, engine?: string | null, launchCommand?: string | null): MeteringVerdict { const name = normalizePaneCommand(engine); if (driver === "terminal") { // Note the engine is IRRELEVANT to the verdict here. A pane holds rendered characters, so @@ -119,7 +125,14 @@ export function classifyEngineMetering(driver: EngineDriver, engine?: string | n }; } if (STRUCTURED_ENGINES.has(name)) { - if (name === "codex") return { metered: true, reason: "Codex exec --json reports each turn's tokens; the observed schema does not report a dollar cost." }; + if (name === "codex") { + // The one engine whose answer the name cannot carry: `codex exec --json` emits per-turn + // tokens, `codex` under any other subcommand emits prose. Same binary, opposite verdict. + if (expectedEngineInvocationMode("codex", launchCommand) === "raw") { + return { metered: false, reason: "This Codex session is not launched as `exec --json`, so it ends a turn with plain stdout and reports no token counts." }; + } + return { metered: true, reason: "Codex exec --json reports each turn's tokens; the observed schema does not report a dollar cost." }; + } return { metered: true, reason: "Claude Code reports each turn's tokens and cost, and that figure is recorded as measured." }; } return { @@ -305,7 +318,13 @@ export async function noteUnmeteredHeadlessDrive( ctx: { userId?: string; instanceId?: string; traceId?: string }, session: { id: string; clientType?: string | null; launchCommand?: string | null }, ): Promise { - if (typeof session.clientType === "string" && expectedEngineInvocationMode(asClient(session.clientType), session.launchCommand) === "structured") return; + // Through the classifier, not around it (#556). The invocation-mode refinement this guard needs + // lives INSIDE `classifyEngineMetering` now, so there is still exactly one place that answers + // "can this reach the ledger". Asking `expectedEngineInvocationMode` directly also had to route + // the engine name through `asClient`, which falls back to "claude" for anything outside its + // four-name list — so aider, opencode, goose, amp, crush and cursor-agent all read as structured + // and recorded nothing, which is the same silence #556 was opened about. + if (classifyEngineMetering("headless", session.clientType, session.launchCommand).metered) return; await noteUnmeteredDrive(env, ctx, { // The runner's own `engineLabel` shape (`:`), rebuilt from the two // fields the cloud holds so the trace names the same thing both sides call it. diff --git a/workers/api/src/lib/prompt-section-estimates.ts b/workers/api/src/lib/prompt-section-estimates.ts index 1de862fd..f2e4344f 100644 --- a/workers/api/src/lib/prompt-section-estimates.ts +++ b/workers/api/src/lib/prompt-section-estimates.ts @@ -1,5 +1,8 @@ import { logEvent } from "./events.js"; -import type { UsageKind } from "./usage.js"; +// From the leaf, NOT from `usage.ts` — `usage.ts` imports `PromptSectionInput` from this file, so +// importing the kind back off it closes a cycle. Type-only, so it erases at runtime and nothing +// would have failed; `import-graph.test.ts` is what catches it. +import type { UsageKind } from "./usage-shape.js"; import type { Env } from "../types.js"; export interface PromptSectionInput { diff --git a/workers/api/src/lib/usage-shape.ts b/workers/api/src/lib/usage-shape.ts index 38abc7a7..c4025e75 100644 --- a/workers/api/src/lib/usage-shape.ts +++ b/workers/api/src/lib/usage-shape.ts @@ -29,6 +29,41 @@ * and it is the only arrangement in which the two sides cannot disagree. */ +/** + * What KIND of call a ledger row records — and the vocabulary `byKind` buckets are keyed by. + * + * Declared in this leaf rather than in `usage.ts` (#302/#556 fix-forward): `usage.ts` needs + * `PromptSectionInput` from `prompt-section-estimates.ts`, and that module needs a kind to label + * its log event with. Both facing the other way is a two-module cycle, type-only and therefore + * invisible at runtime — exactly the shape `import-graph.test.ts` exists to reject, and exactly + * the shape the connector graph was untangled into a leaf to escape. + * + * `usage.ts` re-exports it, so it stays the module you import a kind FROM. Nothing else moved. + */ +export type UsageKind = + | "chat" + | "apply" + | "coding" + /** + * The coding Engine itself (#267) — the CLI child process on the user's machine. + * + * Distinct from "coding", which is the cloud-side Pilot deciding what to instruct it to do. + * Conflating them would hide the split that matters: the Pilot's decisions are cents, the + * Engine's turns are the actual bill. + */ + | "engine" + | "copilot" + | "overseer" + | "run" + | "resume" + | "translate" + | "voice" + // Declarative pipeline LLM step (ai_generate) — e.g. the Outreach agent drafting per lead. + | "pipeline" + // Platform-paid internal AI (issue #44), billed to the platform, not BYOK. + | "embedding" + | "summary"; + /** One slice of a range, in the two units the page reports. */ export interface CoverageSlice { calls: number; diff --git a/workers/api/src/lib/usage.ts b/workers/api/src/lib/usage.ts index 050fa313..53645797 100644 --- a/workers/api/src/lib/usage.ts +++ b/workers/api/src/lib/usage.ts @@ -11,34 +11,17 @@ import { engineSessionFromRowId, engineUsageRowId, type EngineUsageReport } from import { asPayer, CHARGED_SQL, isCharged, payerForEngineAuth, PAYER_LABEL, UNKNOWN_PAYER_KEY } from "./usage-payer.js"; import { bucketLabel, UNASSIGNED_KEY, usageInstanceScopeSql } from "./usage-ids.js"; import { payerCoverage } from "./usage-coverage.js"; -import type { UsageBucket, UsageDay, UsageSummary, UsageTotals } from "./usage-shape.js"; +import type { UsageBucket, UsageDay, UsageKind, UsageSummary, UsageTotals } from "./usage-shape.js"; import type { EngineAuthResolved } from "./usage-payer.js"; import type { PromptSectionInput } from "./prompt-section-estimates.js"; import type { Env } from "../types.js"; -export type UsageKind = - | "chat" - | "apply" - | "coding" - /** - * The coding Engine itself (#267) — the CLI child process on the user's machine. - * - * Distinct from "coding", which is the cloud-side Pilot deciding what to instruct it to do. - * Conflating them would hide the split that matters: the Pilot's decisions are cents, the - * Engine's turns are the actual bill. - */ - | "engine" - | "copilot" - | "overseer" - | "run" - | "resume" - | "translate" - | "voice" - // Declarative pipeline LLM step (ai_generate) — e.g. the Outreach agent drafting per lead. - | "pipeline" - // Platform-paid internal AI (issue #44), billed to the platform, not BYOK. - | "embedding" - | "summary"; +/** + * Re-exported so this stays the module you import a kind FROM; it is DECLARED in `usage-shape.ts`. + * Moving the declaration is what breaks the `usage ⇄ prompt-section-estimates` type-only cycle — + * see the note beside it there for why the leaf is the fix rather than a deferred import. + */ +export type { UsageKind }; /** What a call site knows about the call. provider+model+userId are filled in by * the AI layer (it knows the real model actually used), so callers pass only the