Skip to content

scaffold+broker: dedicated <ns>.balance method for managed budget apps#73

Open
Alexgodoroja wants to merge 1 commit into
pilot-protocol:mainfrom
Alexgodoroja:feat/broker-balance-method
Open

scaffold+broker: dedicated <ns>.balance method for managed budget apps#73
Alexgodoroja wants to merge 1 commit into
pilot-protocol:mainfrom
Alexgodoroja:feat/broker-balance-method

Conversation

@Alexgodoroja

Copy link
Copy Markdown
Collaborator

Problem

Managed (credit-metered) apps meter a per-user $-budget and surface it on the X-Pilot-Credits-Remaining header — but a keyless adapter only returns the response body, so an agent can't actually read that header. There was no first-class way to answer "how much budget do I have left?" before a spend op.

The broker already had half of this: serveCreditBalance + an optional credit.balance_path to shadow a partner's account-balance endpoint (so it returns per-user budget instead of leaking the pooled account). But it was opt-in with no default and no adapter-facing method — dormant.

This PR — wires it end to end

  • Broker: answers a canonical /_pilot/balance for any credit app, from its own per-caller ledger — no upstream call, no master key, no debit, never a 402, scoped to the caller (pooled balance never disclosed). Reuses the existing serveCreditBalance; the canonical path is always available alongside the optional creditBalancePath shadow.
  • Scaffolder: auto-injects <ns>.balance into every auth: managed app (no submission field) — a GET to /_pilot/balance that flows through registration, the manifest exposes list, and <ns>.help like any authored method.
  • scaffold.BalanceMetaPath == broker.pilotBalancePath keeps both ends in sync.

Response body:

{ "balance": "$1.80", "credits_remaining": 1800000, "credits_seed": 5000000,
  "unit": "micro_usd", "scope": "per-pilot-user" }

Tests

  • Broker (zz_balance_meta_test.go): canonical path seeds a fresh caller, reflects a spend, is free, and never forwards upstream.
  • Scaffold (zz_balance_method_test.go): injected for managed apps only (not byo), generated + wired to the reserved path + in the manifest, and the project compiles.
  • Full broker + scaffold + publish suites green.

Docs

MANAGED-KEY.md and PUBLISHING-PLAYBOOK.md document the pattern for all broker apps that meter a budget: set the credit block → the adapter exposes <ns>.balance for free; tell agents to check it (or the header) before a spend.

Managed (credit-metered) apps could meter a per-user $-budget and surface it
on the X-Pilot-Credits-Remaining header, but a keyless adapter only returns the
response body — so an agent could never actually read its balance, and there was
no first-class way to check funds before a spend op.

This wires a dedicated balance method, end to end:

- Broker answers a canonical /_pilot/balance for any credit app from its own
  per-caller ledger — no upstream call, no master key, no debit, never a 402,
  scoped to the caller (the pooled account balance is never disclosed). Reuses
  the existing serveCreditBalance (which also backs the optional creditBalancePath
  partner-endpoint shadow); the canonical path is always available alongside it.
- Scaffolder auto-injects `<ns>.balance` into every `auth: managed` app (no
  submission field), a GET to that path — flowing through registration, the
  manifest `exposes` list, and `<ns>.help` like any authored method.
- scaffold.BalanceMetaPath == broker.pilotBalancePath keeps the two ends in sync.
- Tests: broker (canonical path seeds, reflects spend, free, never forwards) and
  scaffold (injected for managed only, generated + compiles + in the manifest).
- Docs: MANAGED-KEY.md and the publishing playbook document the pattern for all
  broker apps that meter a budget.

@TeoSlayer TeoSlayer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Security + quality review. The core funds-safety design is sound — the <ns>.balance read is provably read-only (no debit, no upstream forward, no master-key touch) and is scoped to the cryptographically-verified caller (the ed25519 pubkey on the signed request, not a spoofable field). But there's a real functional bug to fix before merge, plus rate-limit/test gaps. (Also currently BEHIND base — trivial rebase.)

Functional bug — provisioned apps get a broken (always-403) balance method

The scaffold injection guard is if c.Managed() (internal/scaffold/config.go), which is true for both auth:"managed" and auth:"provisioned". But the broker only serves canonical /_pilot/balance for classic managed apps — provisioned apps route through serveProvisioned() (internal/broker/identity.go), whose switch recognizes only ProvisionSpec.BalancePath (default /_balance) and 403s on anything else. So a newly-scaffolded provisioned app that doesn't hand-author its own .balance will ship a manifest-listed, <ns>.help-documented balance method that always 403s at runtime — exactly the manual step this PR advertises removing. The only provisioned fixture (io.pilot.smol) hand-authors smol.balance → GET /_balance, which masks the bug; no new test exercises auth:"provisioned". MANAGED-KEY.md even notes "provision apps keep their own /_balance route" — the distinction was known but never encoded as a guard.
Fix: gate the injection on something that excludes Provisioned() (or target the app's actual balance path), and add a provisioned-app scaffold test.

LOW / gaps

  • Unrate-limited route: /_pilot/balance is served before the app's Quota/Admit check with no cooldown, so unlike every other method it's uncapped — a free identity can hammer it (store/DB load). It's also intercepted before the app's allow-list with no collision validation.
  • mint_cooldown_ms > 0 interaction: MemStore.Provision's cooldown fires on every repeat call to an existing record, so calling .balance twice within a cooldown window returns 429 're-provision cooldown' instead of the balance — undermining the "check before every spend" pattern the docs recommend. Both new fixtures leave cooldown at 0, so it's untested.
  • No cross-caller isolation test: provision two distinct signed identities against the same app and assert each sees only its own numbers — a meaningful gap for a funds-adjacent authz feature (implementation looks correct, but it should be pinned).
  • scaffold.BalanceMetaPath and broker.pilotBalancePath are two independent /_pilot/balance literals synced only by a "MUST match" comment — extract a shared constant + a cross-package test.
  • Minor: <ns>.help is a hard Validate() error if hand-authored, while <ns>.balance is silently skipped-if-present — inconsistent reserved-name handling.

Docs are otherwise clear and accurate about the read-only / no-402 / no-partner-call / no-master-key guarantees for the classic managed path. Please fix the provisioned-app guard (+ test) and the cooldown/rate-limit interaction, then rebase.

@TeoSlayer

Copy link
Copy Markdown
Contributor

Review fixes implemented in #92 (rebased onto current main, since this branch lives on a fork): provisioned-app balance-403 fix, cooldown-exempt balance reads + light rate limit, shared /_pilot/balance constant, and a cross-caller isolation test. Not merging #92 — pending review.

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.

2 participants