Skip to content

feat(tui): provider-accepted Computer meter receipts - #5781

Merged
Hmbown merged 2 commits into
mainfrom
feat/computer-meter-receipts-20260831
Aug 31, 2026
Merged

feat(tui): provider-accepted Computer meter receipts#5781
Hmbown merged 2 commits into
mainfrom
feat/computer-meter-receipts-20260831

Conversation

@Hmbown

@Hmbown Hmbown commented Aug 31, 2026

Copy link
Copy Markdown
Owner

No-Issue: source-only PRODUCT_PRD §7 Computer meter receipts; no tracked Engine issue yet.

Summary

Source-only v3 Computer metering honesty for Codewhale Engine. Codewhale remains the billing authority; Daytona supplies infrastructure.

  • Bind provider, profile (standard-8 / standard-16 / standard-32), CPU/RAM/disk, 1x/2x/4x multiplier, meter/catalog revisions, account, funding authority, quote identity, and expiry in an immutable admission record before dispatch.
  • Mint receipts only for provider-accepted active seconds, per second. Wall-clock-if-idle, queued, rejected, stopped, suspended, archived, failed-before-acceptance, and teardown-tail cannot enter the entitlement ledger.
  • Provider-observed allocation must exactly equal the admitted profile. The historic 2 vCPU / 4 GiB rollback envelope is refused.
  • Corrections are append-only and preserve the original receipt.
  • cloud_dispatch sandbox create is infrastructure identity, not entitlement; meter_cloud_job requires admission plus a provider-accepted active observation.

No live Daytona, no charge, no deploy.

Testing

  • Targeted: scripts/dev-test.sh crates/tui/src/computer_meter.rs (4/4)
  • Targeted: sandbox_create_is_not_computer_entitlement (1/1)
  • cargo fmt -p codewhale-tui
  • cargo fmt --all -- --check (not run; leaf change)
  • cargo clippy --workspace --all-targets --all-features --locked (not run; leaf change)
  • cargo test --workspace --all-features --locked (not run; leaf change)

Hermetic fixtures live in crates/tui/src/computer_meter/fixtures/cases.json.

Checklist

  • This PR adds a new layer/module/abstraction — it names or deletes the layer it replaces
    • Adds crates/tui/src/computer_meter.rs as the Engine receipt authority. Does not replace cloud_dispatch launch; that remains infrastructure-only.
  • Updated docs or comments as needed
  • Added or updated tests where relevant
  • Verified TUI behavior manually if UI changes (no UI change)
  • Harvested/co-authored credit uses a GitHub numeric noreply address (N/A; authored as CodeWhale Bot)

Signed-off-by: CodeWhale Bot bot@codewhale.net

Bind v3 Computer admission (profile, 1x/2x/4x multiplier, account,
quote, expiry) before dispatch. Mint immutable receipts only for
provider-accepted active seconds at per-second granularity. Reject
wall-clock-if-idle, queued/stopped/teardown time, and allocations
that do not exactly match standard-8/16/32.

Tests use hermetic Daytona-shaped fixtures. No live provider call,
no charge, no deploy.

Signed-off-by: CodeWhale Bot <bot@codewhale.net>
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@claude

claude Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Claude finished @Hmbown's task in 3m 11s —— View job


Review complete

  • Gather context (PR diff, changed files)
  • Read crates/tui/src/computer_meter.rs
  • Read crates/tui/src/computer_meter/tests.rs and fixtures
  • Read crates/tui/src/cloud_dispatch.rs diff
  • Check lib.rs module wiring
  • Check for prior/related modules (model_*, provider_*, meter/billing) per AGENTS.md
  • Attempt compile-check (blocked, see note below)
  • Post review feedback

This is a source-only, additive module (crates/tui/src/computer_meter.rs), consistent with the PR description — no other production code calls issue_computer_meter_receipt / meter_cloud_job yet, so blast radius is limited to the module itself and its tests. No model_*/provider_*/config-mirroring collisions found; this doesn't touch LLM provider/route selection (EPIC #2608 invariant N/A here — it's about the Daytona infra provider, not model routing).

Note: I wasn't able to run cargo check/cargo test/cargo fmt in this environment — the sandbox required approval for build commands that wasn't available non-interactively. Findings below are from static reading of the diff; no .unwrap()/.expect() exist outside #[cfg(test)] blocks in either changed file, so no obvious panic paths.

Findings

1. Admission expiry only checks interval start, not endcrates/tui/src/computer_meter.rs:460-463

let expires = parse_timestamp(&admission.expires_at, "expiresAt")?;
if started >= expires {
    return Err(ComputerMeterError::AdmissionExpired);
}

Only started_at is compared against admission.expires_at. If a provider-accepted interval starts just before expiry but ends well after it (ended_at > expires_at), the receipt still mints accepted_seconds for the entire interval, including time after the admission expired. Given the module's stated purpose ("Admission expiry (inclusive bound is refused)" and "Engine refuses to meter anything that is not this record"), this looks like a real gap rather than intended behavior — an expired admission can still fund active seconds past its own expiry. No fixture in computer_meter/fixtures/cases.json exercises computer_admission_expired at all, let alone an interval that straddles expiry.
Fix this →

2. sum_standard_equivalent_seconds has no lineage awareness — corrections can double-countcrates/tui/src/computer_meter.rs:568-575 (vs. correct_computer_meter_receipt, lines 542-566)

pub fn sum_standard_equivalent_seconds(receipts: &[ComputerMeterReceipt]) -> u64 {
    receipts.iter().map(|r| r.standard_equivalent_seconds).fold(0, u64::saturating_add)
}

correct_computer_meter_receipt explicitly preserves the original receipt (append-only) and stamps the new one with correction_of/lineage. But sum_standard_equivalent_seconds blindly sums whatever slice it's given — if a caller (billing rollup) passes both the original and its correction, the corrected interval is counted twice. The doc comment says "across independent Computer receipts," implying the caller must pre-filter, but there's no helper or check enforcing that, and it's untested (the one test that exercises corrections never sums alongside the original). Given this whole PR is about billing/entitlement honesty, this silent double-count risk seems worth closing or at least asserting against.
Fix this →

Minor / lower-confidence notes

  • issue_computer_meter_receipt compares observation.provider to admission.provider with exact string equality (computer_meter.rs:468), while profile ids are case/whitespace-normalized via normalize_token. If provider strings ever originate from different casing sources this would spuriously reject; likely fine today since both sides currently use the literal "daytona".
  • Good invariant coverage otherwise: undersized/oversized "rollback" allocation mismatches, wall-clock/idle, queued/stopped/teardown non-accepted states, and historic-profile inadmissibility are all fixture-tested with stable error codes.
  • cloud_dispatch.rs keeps meter_cloud_job correctly separate from create_sandbox (infra id ≠ entitlement), matching the module doc's own framing.

Signed-off-by: CodeWhale Bot <bot@codewhale.net>
@claude

claude Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Claude finished @Hmbown's task in 2m 1s —— View job


Review complete

  • Gather context (diff vs previous review, changed files)
  • Re-read crates/tui/src/computer_meter.rs
  • Re-read crates/tui/src/computer_meter/tests.rs and fixtures
  • Check cloud_dispatch.rs diff
  • Check lib.rs module wiring
  • Confirm whether prior findings were addressed
  • Post review feedback

This "synchronize" trigger only adds commit 45f54ec (a changelog entry for an unrelated PR #5776, CHANGELOG.md / crates/tui/CHANGELOG.md only). The Computer-meter code itself (computer_meter.rs, its tests.rs/fixtures, and cloud_dispatch.rs) is byte-identical to the state already reviewed in the earlier run on this PR — I independently re-verified the module and confirm both previously-flagged issues are still present and unaddressed:

Findings

1. Admission expiry only checks interval start, not endcrates/tui/src/computer_meter.rs:460-463

let expires = parse_timestamp(&admission.expires_at, "expiresAt")?;
if started >= expires {
    return Err(ComputerMeterError::AdmissionExpired);
}

Only started_at is compared to admission.expires_at. An observation whose ended_at falls after expiry still accrues accepted_seconds/standard_equivalent_seconds for the full interval, including time after the admission expired — the opposite of the module's own "Engine refuses to meter anything that is not this record" framing. No fixture in fixtures/cases.json exercises computer_admission_expired at all (confirmed by grep — every expiresAt in the fixture pack is 2026-08-31T18:00:00.000Z and no observation interval crosses or precedes it), so this path is entirely untested.
Fix this →

2. sum_standard_equivalent_seconds has no lineage awareness — corrections can double-countcrates/tui/src/computer_meter.rs:568-575

pub fn sum_standard_equivalent_seconds(receipts: &[ComputerMeterReceipt]) -> u64 {
    receipts.iter().map(|r| r.standard_equivalent_seconds).fold(0, u64::saturating_add)
}

correct_computer_meter_receipt preserves the original receipt and stamps the new one with correction_of/lineage (append-only), but nothing stops a caller from summing both the original and its correction — the corrected interval would be billed twice. tests.rs's "correction" case never sums alongside the original, so this is untested. Given the whole PR is about billing honesty, this silent double-count risk is worth closing (filter by correction_of/lineage before summing) or explicitly asserting against in the helper.
Fix this →

Additional observation (new, not in the earlier review)

3. Sandbox↔admission binding is enforced only by the cloud_dispatch::meter_cloud_job caller, not by issue_computer_meter_receipt itselfcrates/tui/src/computer_meter.rs:447-519 vs. crates/tui/src/cloud_dispatch.rs:677-691. ComputerAdmission carries no sandbox/allocation id (it's bound pre-dispatch, before a sandbox exists, which is reasonable), and issue_computer_meter_receipt never checks observation.provider_sandbox_id against anything on the admission — only meter_cloud_job layers on the job.sandbox_id == observation.provider_sandbox_id check. Any other call site (or a future one) that invokes issue_computer_meter_receipt directly could mint a receipt against any provider-accepted observation whose provider/CPU/RAM/disk happen to match the admitted profile, regardless of which sandbox actually ran it. Not necessarily a bug today (only one call site exists, and it does the check), but worth a doc comment on issue_computer_meter_receipt making clear that sandbox-identity binding is a caller obligation, so it isn't silently dropped by a future integration.

Good choices

  • Provider/model route safety (EPIC v0.8.65 EPIC: Separate provider facts, model facts, offerings, and route resolution #2608): not applicable — this is the Daytona infrastructure provider, not LLM provider/model routing; no prefix-based inference present.
  • Solid fixture coverage otherwise: undersized/oversized allocation "rollback" mismatches, wall-clock/idle, queued/stopped/teardown non-accepted states, and historic-profile inadmissibility are all covered with stable error codes.
  • cloud_dispatch.rs correctly keeps meter_cloud_job separate from create_sandbox (infra id ≠ entitlement), and credential/error messages are scrubbed of secrets (tested).
  • No new .unwrap()/.expect() outside test code in either changed file.

@codewhale-agent codewhale-agent 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.

Codewhale review

The PR adds a new computer_meter module that binds immutable admission records and mints receipts only for provider-accepted active seconds. It includes fixture-based tests and a cloud_dispatch helper. The core design is sound, but an admission expiry boundary check is incomplete and the PR includes unrelated changelog entries.

Findings

  • [ERROR] Admission expiry is not enforced for intervals that cross the expiry boundary (crates/tui/src/computer_meter.rs)
    In issue_computer_meter_receipt, only the interval start is checked against admission.expires_at. A provider-accepted interval that begins before expiry but ends after expiry is billed for the entire duration, including seconds after admission expiry. This can over-bill the customer and contradicts the admission expiry contract. The function should fail closed when ended_at is after expires_at, or truncate the interval before billing.
  • [WARNING] Unrelated changelog entries included in PR (CHANGELOG.md)
    Both CHANGELOG.md and crates/tui/CHANGELOG.md add a Pod-language entry referencing #5776. These entries appear unrelated to Computer meter receipts and likely came from another branch. They should be removed to keep this PR focused and avoid misleading release notes.
  • [INFO] Missing test for cloud job sandbox id mismatch (crates/tui/src/cloud_dispatch.rs)
    meter_cloud_job is a public wrapper that rejects mismatched job.sandbox_id, but the new sandbox_create_is_not_computer_entitlement test only covers a matching sandbox and idle/accepted observations. A test should verify that a mismatched sandbox id fails closed with computer_meter_allocation_mismatch.
  • [INFO] Several error variants and correction error paths are not tested (crates/tui/src/computer_meter/tests.rs)
    The fixture pack covers many happy-path and failure cases, but there are no tests for AdmissionExpired, IntervalReversed, RevisionMismatch, ReferenceInvalid, TimestampInvalid, or correct_computer_meter_receipt error branches such as account/admission mismatch or profile/multiplier differences. Adding at least the expiry-crossing test would also strengthen confidence in issue #5781.

Assessment

The module is well-structured and the receipt flow is mostly fail-closed, but the incomplete expiry-boundary enforcement is a billing-honesty bug that should be fixed before merge. Unrelated changelog entries should also be removed. Additional negative tests would materially improve coverage.


Advisory review by Codewhale (codewhale review --pr 5781 --post, head 45f54eceaa18037930d453282e95d81dc23ce484). Line-specific findings are also posted as inline review comments; mechanical fixes arrive as committable suggestions you can apply from the Files tab. CODEOWNERS approval still governs merge.

@Hmbown
Hmbown enabled auto-merge (squash) August 31, 2026 21:28
@Hmbown
Hmbown merged commit 7db8b47 into main Aug 31, 2026
28 checks passed
@Hmbown
Hmbown deleted the feat/computer-meter-receipts-20260831 branch August 31, 2026 21:51
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