fix(usage): record aborted auxiliary model calls as usage-unknown, not zero

[ggbdpq]

Summary

Part 3 of #5691 (the abort-path accuracy gap; we hold the issue, design note posted as issuecomment-5915690378). Auxiliary Host model calls (goal evaluation, session titles, WorkHub routing, recaps, …) recorded every abort, provider error, and usage-less completion as inputTokens: 0, outputTokens: 0 in the legacy table — a zero that reads as "genuinely free", exactly the ambiguity the canonical ledger was built to remove.

On this path usage can never be partial today (an aborted tool-free call throws without usage), so usage-unknown is the only honest value. The canonical vocabulary already exists — ModelCallUsageBasis ('reported'|'partial'|'missing'), a schema CHECK forbidding token counts on missing-usage rows, read-side coverage SUMs — but its writer is reachable only from an AgentRun, and auxiliary calls run outside any run.

The seam (disclosed for maintainer adjudication): ModelCallLedger / InteractiveUsageStoresWriter.modelCalls gain recordUsageUnknownAttempt() — the one ledger write that does not come from the run event stream, exposed through the stores facade the same way the existing canonical read channel (server/canonical-usage-reader.ts) is. runHostAuxiliaryModelCall's failure catch and zero-usage success path migrate onto it (rows land usageBasis: 'missing' / costBasis: 'unpriced', upsert-keyed by attemptId, publishing the owning Session); calls with real provider usage keep the existing path.

Out of scope, documented not changed: exposing unknown-usage counts over the wire to Desktop (touches protocol/index.ts + overview payload — can follow once this lands), and createHostPluginModel's callKind: 'main' routing through the auxiliary pipeline (flagged for a separate decision). So this is Refs #5691, not Fixes.

Verification

Check	Result
Red A (tests only, no implementation)	tsc exit 2 — recordUsageUnknownAttempt missing on the writer type
Red B = ablation (seam kept, authority migration reverted)	runtime-host focused exit 1: the two core canonical assertions fail again (7 fails = 5 pre-existing + 2 core)
Green	storage focused 59 pass / 0 fail / 1 skip (incl. new seam cases); runtime-host focused 34 pass / 5 fail / 1 skip — the 5 fails are pre-existing, name-identical to the before-baseline
core focused (usage-ledger-merge + model-call-attempt)	31/31
Build chain core→storage→runtime→runtime-host	exit 0
epoch guard --base 9b089f58e	exit 0 — no protocol changes, no epoch bump
biome / asf-headers / git diff --check	all clean

Honest boundaries: 5 runtime-host fails are pre-existing environment failures (fast ~80-320ms fails, name-identical across before/after baselines); a pre-existing hung-handle issue required --test-force-exit locally — CI is the final arbiter. Production diff is 176 lines across 3 files (under the 250-line minimal-core budget); the rest is tests (204 lines extend execution-model-composition.test.ts, whose existing call-2 abort assertion previously pinned the buggy zero-zero behavior and now pins the canonical row).

Fixes the record half of #5691 · Refs #1679

Generated-by: GLM-5.3-Flash (ZCode)

Settlement narrowing (disclosed for review)

The two mid-review CI failures were this fix's own recording interacting with hosted-execution settlement: the wire-contract test's session-title auxiliary call always fails against the test provider (platform-independent; main records it as a legacy 0/0 row and settles), while this branch records it as a canonical usage-missing row — and settlement refuses any range holding usage-missing rows. Full chain and fix in the CI comment.

Settlement no longer reads the ledger-wide coverage field (the first narrowing attempt touched the shared COVERAGE_SUMS, which the pricing coordinator and inspector also consume — reviewer P2): COVERAGE_SUMS is restored to counting every usage-missing row, and hosted settlement checks a new run-scoped aggregation (RUN_SETTLEMENT_COVERAGE_SUMS, keyed by the execution's session and excluding the auxiliary no-run sentinel turn). An auxiliary failure is honest accounting but nobody's unsettled obligation; a run's own dispatch without usage still refuses settlement; Desktop unknown totals stay visible. usage.query shapes unchanged. Fallback if maintainers prefer range-strict settlement: revert the recording (abort/failed included) to legacy wholesale and keep only the seam — a clean local chain through d917117 supports either outcome.

Checklist

• I have read the Contributing Guide
• Tests cover the changed behavior (red → green → ablation)
• Seam shape disclosed for maintainer adjudication before merge

A schema-10 usage database opened by an older build is refused as newer-than-supported, matching every earlier schema bump.

[ggbdpq]

First CI round failed on two tests; both characterized:

1. public-entrypoints (fixed at d591f5bc3): the exact-in-both-directions guard flags a stale snapshot entry — the usage-unknown seam gives @maka/storage/model-call-ledger its first external consumer since repairPendingModelCallProjections lost its last caller (the exact history the snapshot's own comment records), so the entry left PREEXISTING_UNCONSUMED_ENTRYPOINTS. Removed the entry and updated the comment; guard reruns green locally (node --test public-entrypoints.test.js exit 0 after rebuild).

2. acp-goal-plan-child-process 30s timeout: environment flake, not a regression. Single-run locally at head: 7/7 pass in 13.7s — comfortably inside the 30s budget this test's harness got in test(cli): give the plan-approval execution subtest a 30-second harness budget #5841 precisely because it's a borderline-flake veteran, and faster than the 16.4s local baseline recorded then. The failure ran the full plan/paging/revision flow correctly (slow, not wrong) on a queued runner. Also mechanically unlikely to be this PR: the migrated paths only change how abort/error/usage-less completions are recorded; calls with provider usage keep the existing write path, so the steady-state call path gains no IO.

Re-run should be green on both.

Generated-by: GLM-5.3-Flash (ZCode)

[hqhq1025]

I reviewed the exact head d591f5bc3ea54d7e4aac4d0d25791b79328dc2b1 and found no substantiated P0–P3 issue in the six-file change. The auxiliary-call failure/abort and usage-less completion paths now write a canonical usageBasis: missing row rather than a misleading zero-token legacy row. The ledger binds null token/cost fields, upserts by attempt ID, and publishes the owning Session after the write (execution-model-authority.ts, model-call-ledger.ts, usage-stores.ts). Calls with reported usage still use the existing legacy path.

Node 24 build:test, the storage usage-stores suite (41/41), and the two focused Host auxiliary tests (2/2) passed locally. git diff --check and merge-tree against current main (c838e1fa) were clean. Hosted test was pending at review time. I did not run the full Host suite or a real provider abort. The PR explicitly leaves Desktop's legacy usage display and reported auxiliary calls outside the canonical ledger; this review does not validate those separate follow-ups or merge readiness.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[ggbdpq]

Second and third CI rounds traced to a real interaction between this fix's recording and hosted-execution settlement — root-caused with a local diagnostic print (the CI log doesn't include failureReason), fixed at 551efbc34 + d917117c5:

The chain: the headless coding wire-contract test's session-title auxiliary call always fails against the test provider (responses wire vs non-streaming auxiliary call → Invalid JSON response — platform-independent, happens on main too). Main records that failure as a legacy 0/0 row and settles. This branch's abort/failed recording — the core of #5691 — records a canonical usage-missing row, and hosted-execution settlement refuses any queried range whose coverage holds usage-missing rows (missing_attempt_usage), so the execution could never settle. Confirmed by a disable-the-write ablation: with the fail-path write off, the failure reverts to that test's pre-existing local hash assertion.

The fix (two parts):

1. 551efbc34 — the zero-usage success path (provider answers without usage) goes back to the legacy 0/0 row: it was beyond the issue's "aborted or failed" wording, and it's the piece whose right shape depends on the settlement semantics below.
2. d917117c5 — narrow settlement coverage to run-owned rows: usageMissingAttempts now counts usage-missing rows whose turn is not the shared no-run sentinel auxiliary calls record under. An auxiliary failure is honest accounting but nobody's unsettled obligation; a run's own dispatch coming home without usage still refuses settlement, unchanged. The Usage screen computes coverage from its own SQL and keeps counting every row — Desktop unknown totals unchanged. usage.query shapes unchanged (epoch guard clean).

Disclosed for review: this is a settlement-semantics narrowing — if maintainers would rather keep settlement range-strict and revisit #5691's recording separately, the fallback is reverting the recording to legacy wholesale and keeping only the seam; the local chain is clean through d917117c5.

Re-verified at d917117c5: the headless test's settlement assertion passes (its remaining local failure is the pre-existing hash mismatch, which matches on CI); new storage case settlement coverage ignores usage-unknown rows recorded outside any run red → green → ablation red; storage 60/0/1skip; core 31/31; runtime-host focused 34 pass / 5 pre-existing (name-identical to baseline).

Generated-by: GLM-5.3-Flash (ZCode)

[hqhq1025]

I reviewed the exact head d917117c5cac32a14ea3df7047a07b836d59e426, including the increment since my earlier review. The new commits restore legacy zero-token recording for usage-less successful auxiliary calls and exclude auxiliary sentinel rows from the canonical ledger's shared missing-usage coverage to let hosted executions settle. I found one P2 in that shared coverage change, left inline. The earlier review applies only to d591f5bc, not this head.

Node 24 build:test, focused storage/query tests (54/54), and the two focused Host auxiliary tests (2/2) passed. git diff --check and merge-tree against current main (c838e1fa) were clean. Hosted test was pending at review time. I did not run a real provider abort or a packaged Desktop; the P2 makes this head not ready to merge on the evidence reviewed.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[hqhq1025]

Reviewed exact head 0e3f82923960307402ab4b154e07d551bd8a9e31. The earlier P2 is fixed for normal hosted IDs: ledger-wide COVERAGE_SUMS again counts auxiliary usage-unknown attempts (packages/storage/src/model-call-usage-sql.ts:65-71), while hosted settlement reads an in-process run-scoped count (packages/runtime-host/src/server/hosted-execution-runner.ts:94-96). I found one remaining P2 at the run/auxiliary identity boundary, detailed inline.

Node 24 workspace build:test, 64 focused storage/runner tests, and 3 focused Host auxiliary tests passed. The current-head hosted test is green; a fresh-main merge-tree and git diff --check are clean. I also reproduced the identity collision against the built ledger: a missing-usage row with sessionId=turnId="auxiliary" gives public usageMissingAttempts=1 but runSettlementCoverage("auxiliary")={attempts:0,usageMissingAttempts:0,usagePartialAttempts:0}. I did not run a real provider abort, packaged Desktop, or the full local suite. This head is not merge-ready until the collision is addressed.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[hqhq1025]

Reviewed 185482af298215218f68589f19103362c27331b8, with fresh main 1e80e3b885d963b799f7e2e9a70083ceda99943d. One substantiated P2 remains in the migration; this head is not ready to merge.

• [P2] The no_run backfill still infers ownership from turn_id = 'auxiliary', overwriting legitimate run-owned rows (packages/storage/src/sqlite-usage-schema.ts:236). I projected a missing-usage attempt through the canonical AgentRun authority for a legally named auxiliary execution. With a current schema-10 database, ordinary close/reopen preserves no_run = 0 and settlement missing count 1. Upgrading the corresponding schema-9 table, or reopening schema 10 when another operational schema scope needs migration, changes the same row to no_run = 1; settlement then reports zero attempts and zero missing while public coverage still reports one missing attempt. The latter path happens because the operational migration invokes every scope's migrator (packages/storage/src/operational-state-store.ts:483). runSettlementCoverage excludes the rewritten row (packages/storage/src/model-call-ledger.ts:399), reintroducing the ownership collision this head otherwise fixes. The inline finding includes the required correction.

The underlying changes are otherwise carried through: auxiliary abort/error paths write canonical missing-usage records instead of legacy zero-token calls (packages/runtime-host/src/server/execution-model-authority.ts:735); public coverage again counts every missing-usage row. Hosted settlement uses a separate Session-scoped query (packages/runtime-host/src/server/hosted-execution-runner.ts:95), and new rows derive no_run from the writer, so the fresh-database sentinel-collision test passes. The usage schema moves from 9 to 10; no protocol surface change is introduced.

Validation with Node 24.18.1: build:test, storage/query/schema/public-entrypoint tests 68/68, hosted-runner tests 9/9, auxiliary composition tests 3/3, and current-main synthetic merge / git diff --check pass. The independent migration reproduction uses real authority projection and the normal ledger-open migration path; its current-schema reopen control passes, while both migration scenarios reproduce the incorrect ownership and counts. The existing regression tests do not cover these upgrades. Current-head hosted test is green; that does not cover this failure.

Scope limits: I did not run a real provider abort, packaged Desktop/Host, the full Host suite, or an end-to-end hosted execution spanning an upgrade. The reproduced result is the corrupted persisted ownership and false-zero settlement query, not an observed production environment release. Successful auxiliary completions without reported usage still use the legacy zero-token path (packages/runtime-host/src/server/execution-model-authority.ts:810); that residual behavior is unchanged here and remains outside the abort/error correction.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[Astro-Han]

Incremental review of exact head 6929b9624b74cb1c073caa459f4e93df2e6471a8 against previous head 185482af2 (fast-forward; range-diff shows earlier commits unchanged) and main 1e80e3b8.

What changed since 185482a: two commits touching only packages/storage/src/sqlite-usage-schema.ts (+5/-9). The UPDATE usage_model_call_attempts SET no_run = 1 WHERE turn_id = 'auxiliary' AND no_run = 0 backfill and its NO_RUN_TURN_ID import are removed; no_run is now added only through ensureColumn(..., 'INTEGER NOT NULL DEFAULT 0') (sqlite-usage-schema.ts:235).

Earlier P2 (review 5374520666) is fixed. Ownership is now written only by the two binders: the event-stream projection writes no_run: 0 (model-call-ledger.ts:554), and recordUsageUnknownAttempt writes no_run: 1 (model-call-ledger.ts:594). Nothing infers it from turn_id. A schema-9 upgrade defaults existing rows to 0. That is correct: main has no usage-unknown or NO_RUN_TURN_ID writer (git grep on origin/main finds none), so any pre-column turn_id='auxiliary' row comes from a real hosted execution and belongs to that run. ensureColumn returns early when the column exists, and nothing in the migration writes no_run anymore. Re-running the migration at schema 10 (close/reopen, or another scope triggering it) therefore leaves both run-owned rows and true no-run rows as they are. The blob-spread path still writes 0 AS no_run (sqlite-usage-schema.ts:382). This is correct because those rows predate the usage-unknown seam. runSettlementCoverage filters on no_run = 0 (model-call-ledger.ts:399). The regression test for a hosted execution literally named auxiliary is still in place (model-call-usage-query.test.ts:229-270).

Merge/CI: merges cleanly with origin/main (git merge-tree); GitHub reports MERGEABLE; the test check passes. No protocol changes.

Findings: no substantiated P0-P3 issues at this head.

Not exercised: I did not run tests locally because the review host's disk was full, which blocked npm ci. I verified the change by reading the code and checking every no_run writer and reader. There is no dedicated migration test that seeds a schema-9 database with a run-owned turn_id='auxiliary' row and asserts no_run stays 0 across repeated migrations. With the UPDATE removed, that case is correct by construction.

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

[ggbdpq]

Gentle ping — this has been quiet for a week. All three review rounds are addressed: the last P2 (the no_run backfill inferring ownership from turn_id = 'auxiliary') was fixed by removing the backfill outright so ownership stays recorder-owned, and the Oct 1 incremental review at the current head 6929b9624 found no substantiated P0-P3 issues. The test check is green. Would you be able to take another look? Thanks!

[liugddx]

Reviewed exact head 6929b9624. The direction is right, and the previous rounds' findings are resolved. The turn_id = 'auxiliary' backfill is gone: no_run is now written only by the projection (0, model-call-ledger.ts:554) and by recordUsageUnknownAttempt (1, :594), and the migration only adds the column with default 0. A simulated schema-9 database upgrades to 10, passes assertCurrentOperationalTargetSchema and is idempotent on rerun. The branch still merges cleanly with current main, which now includes #5709. The two compose correctly: auxiliary usage-unknown rows keep their own call_kind, so the main-only Inspector view excludes them.

P2 (land before merge): the completeness check and the totals it guards now read different scopes. In hosted-execution-runner.ts:95-96, the settled totals come from #readUsage (:159-168). That is a usage.query summary over the time window with no session predicate, so it counts every Session in the window, including child Sessions the hosted run spawns. The new completeness check, runSettlementCoverage (model-call-ledger.ts:393-407), filters session_id = executionId AND no_run = 0, so it sees only the root Session. Before this PR, both read the same window. Scenario:

1. A hosted execution delegates to a subagent.
2. One of the child's model calls records usage_basis = 'missing' (or 'partial').
3. Settlement now returns settled with undercounted tokens, and possibly a non-null, undercounted costUsd for partial rows, where it previously returned indeterminate: missing_attempt_usage.

Fix: scope runSettlementCoverage to the same window as #readUsage (completed_at BETWEEN from AND to) and keep AND no_run = 0, dropping the session filter. Pass startedAt/now through instead of executionId. That still excludes the run-less auxiliary rows this PR is about, without un-guarding the child runs. A test with a child-session missing row in the window would pin it.

P3, non-blocking.

• RunSettlementCoverage.attempts is computed but never read. Drop it, or assert it in a test.
• No test covers the schema 9 → 10 upgrade path. Its DDL signature only matches because SQLite's ADD COLUMN places the column the same way a fresh create does, which is worth pinning.
• A schema-10 database opened by an older build is refused as "newer than supported". That matches earlier bumps, but it deserves a line in the description.

Locally, the storage usage, schema and operational-state suites pass 110/0 (2 skipped). The hosted-runner and execution-composition suites show 6 failures, and the same 6 fail at the merge-base: a wire-contract hash, a Turn that never terminates, and 4 Windows EBUSY errors. None come from this PR. I didn't run a live hosted execution with a child run, so the P2 scenario is traced from code, not reproduced.

Happy to approve once the window scoping lands.

[ggbdpq]

Thank you - the window-scoping P2 and all three P3s are addressed at head 5993466:

• P2: runSettlementCoverage now takes the settlement window (startedAt, the settled-at instant) instead of the execution id, and its SQL mirrors countableFilter's window exactly (completed_at >= from AND completed_at <= to) with no_run = 0 kept - the same scope as the #readUsage totals it guards, so a delegated child Session's missing or partial usage can no longer settle as complete.
• P3s: the computed-but-unread attempts field is dropped; the schema 9 -> 10 upgrade path is pinned by a test (full operational target, usage regressed to schema 9, column restored in place with its recorder-owned default, assertCurrentOperationalTargetSchema green, rerun idempotent); the PR description now states that a schema-10 database opened by an older build is refused.

Verification: the child-session missing-row pin you sketched is in (run settlement coverage reads the window, not one session), plus a mutation check - re-scoping the SQL back to one session flips exactly the three window tests red. Storage usage/schema/stores suites 65/65; hosted-execution-runner 9/9 and execution-composition 46/46 at this head (the six failures you observed did not reproduce on this machine - macOS arm64, Node 24); biome clean on the ten touched files; check:asf-headers pass.

Ready for re-review when you are.

[Astro-Han]

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental review of 6929b962 -> 5993466f44940c9e2689653428eeef10073c287d. This is a fast-forward with one new commit (5993466), and the merge-base with main (9b089f58) is unchanged, so the whole delta is the author's.

Prior findings. My review of 6929b96 had no open findings. liugddx's review of the same head (5459689671) raised one P2 and three P3s. All four are addressed:

• P2 (the settlement check and the totals it guards read different scopes): fixed. runSettlementCoverage(from, to) now filters cost_basis IS NOT NULL AND no_run = 0 AND completed_at >= ? AND completed_at <= ? (model-call-ledger.ts:394-408). That is exactly countableFilter (model-call-usage-sql.ts:108-113) plus no_run = 0. resolveUsageRange passes an object range through unchanged, and the runner computes settledAt once and uses it for both #readUsage and coverage (hosted-execution-runner.ts:94-96). So the check now covers the same rows as the totals, delegated child Sessions included, minus only the usage-unknown sentinel rows. Compared with main, where the check read the usage.query coverage over the same window, the only difference is that no_run = 1 rows are excluded, which is what this PR is for. Rows from unrelated concurrent Sessions in the window still count, as they do on main, so there is no regression there.
• P3 (unused attempts): removed from RunSettlementCoverage and RUN_SETTLEMENT_COVERAGE_SUMS.
• P3 (no 9->10 upgrade test): sqlite-usage-schema.test.ts:344-377 drops no_run from a full target, re-migrates, checks assertCurrentOperationalTargetSchema and NOT NULL DEFAULT 0, and re-runs to confirm idempotency.
• P3 (older build refuses a schema-10 database): the PR description now states this.

The new storage test run settlement coverage reads the window, not one session pins both sides of the window: a child Session's in-window missing row counts, and a row outside the window does not.

Findings

• [P3] The runner-level stub settlementCoverage (hosted-execution-runner.test.ts:384) ignores its (from, to) arguments, so no test checks that the runner passes the same window to coverage and to #readUsage. The current code is correct. A one-line assertion in the stub would keep a later edit from quietly separating the two windows again. Non-blocking.

No P0-P2 findings.

Schema/epoch. The usage schema moves 9 -> 10. Main is still at 9 (sqlite-usage-schema.ts:22 and operational-target-schema.ts are untouched since the merge-base), so there is no version collision. No protocol or wire files changed in this delta.

Verification. In a worktree on the head, built core/storage/mcp/runtime/plugins/runtime-host (tsc clean). Storage model-call-usage-query, sqlite-usage-schema and usage-stores pass 65/65. Runtime-host hosted-execution-runner passes 9/9. Not run: the full workspace suite or a live hosted execution with a child run.

Mergeability/CI. GitHub reports MERGEABLE, and git merge-tree against current main (74094c63) is clean. CI test passes on 5993466. The merge state is BLOCKED only on required review.

Verdict: ready from my side.

[liugddx]

Approving exact head 5993466f4. The P2 is fixed the right way. runSettlementCoverage(from, to) now uses the same window as the totals it guards: one settledAt feeds both #readUsage and the coverage, and the filter is countableFilter's window plus no_run = 0. A delegated child Session's missing usage blocks settlement again, while the run-less auxiliary sentinel rows stay excluded, which is the point of this PR. The new storage test pins both sides of the window: an in-window child row counts and an out-of-window row does not. The 9 → 10 upgrade test, the dropped attempts field and the description note close the three P3s.

Merges cleanly with current main (74094c632, which includes #5709), and CI is green on this head. Astro-Han's runner-stub P3, asserting that the stub receives the same (from, to), would be a good follow-up but doesn't block.

Thanks @ggbdpq. With #5709 and this landed, #5691's usage split is complete.