Skip to content

fix(node): Skip payment for free services#606

Merged
kotevcode merged 1 commit into
mainfrom
fix/free-service-payment-gating
May 26, 2026
Merged

fix(node): Skip payment for free services#606
kotevcode merged 1 commit into
mainfrom
fix/free-service-payment-gating

Conversation

@kotevcode
Copy link
Copy Markdown
Contributor

Description

Fixes seller-side payment gating so requests for resolved-free services bypass all payment checks. This prevents free services from returning 402 Payment Required when payments are enabled or when a buyer has an existing exhausted paid channel with the peer.

Release Notes

  • Fixed free peer services incorrectly returning 402 Payment Required in some payment-enabled seller flows.
  • Added regression coverage for first-time buyers using free services without deposits.

Types of Changes

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Chore (maintenance tasks, refactoring, or non-functional changes)

Checklist

  • My code follows the code style of this project
  • I have added the necessary documentation (if appropriate)
  • I have added tests (if appropriate)
  • Lint and unit tests pass locally with my changes

@kotevcode
Copy link
Copy Markdown
Contributor Author

@claude

@claude
Copy link
Copy Markdown

claude Bot commented May 25, 2026

Claude finished @kotevcode's task in 1m 51s —— View job


Code Review

  • Gather context from PR
  • Read changed files
  • Analyze diff
  • Post review feedback

Overall this is a clean, well-scoped fix. The logic is sound and the test coverage is thorough. A few observations below:


seller-request-handler.ts

Hoisting resolveProviderPricing() — good refactor

Previously the pricing was computed 2–3 times at different points in the handler. Hoisting it to the top (lines 104–105) and computing isFreeService once is cleaner and eliminates redundant work. The old code also had a defensive providerPricing ? ... : false null check that was unnecessary given the function's non-nullable return type — that's correctly removed.

_isFreePricing() helper is correct

The helper (lines 513–518) correctly extends the free check to include cachedInputUsdPerMillion:

const cachedPrice = pricing.cachedInputUsdPerMillion ?? pricing.inputUsdPerMillion;

The fallback to inputUsdPerMillion means an unset cached price inherits the input price — and since inputUsdPerMillion === 0 is already required by the prior check, a service is only non-free if cachedInputUsdPerMillion is explicitly set to a non-zero value. This is the right semantics.

Minor: The parameter uses an inline import(...) type rather than a top-level import. It's consistent with the existing resolveProviderPricing() signature in the same file, so it's not a blocker, but consolidating to a named import at the top of the file would be cleaner if the file is touched again.

Budget exhaustion bypass (line 155)

if (spm && !isFreeService) {

This is the core regression fix. An exhausted paid channel can no longer block a free service request. The comment is clear on the intent. Correct.

NeedAuth/recordSpend bypass (line 372)

if (!isFreeService && spm?.hasSession(buyerPeerId)) {

Correct. Without this guard, recordSpend would be called with costUsdc = 0n on every free request, accumulating zero-cost entries in the payment state unnecessarily, and sendNeedAuth would fire with lastRequestCost: '0' — neither is harmful, but both are semantically wrong and the old test was validating that wrong behavior. Good catch.

Metering still records for free services

sessionTracker.recordMetering() is not guarded by !isFreeService, which is intentional — free services should still contribute to analytics and load tracking. This is the right call.


node-payment-pricing.test.ts

The renamed tests now accurately reflect what they're testing. The two new tests cover the important scenarios:

  1. skips the first-time buyer 402 when the requested service has a free override on a paid provider — exercises the service pricing override path through resolveProviderPricing(). This is the most critical scenario because it tests a paid provider (inputUsdPerMillion: 10) where only a specific service is free, ensuring _isFreePricing is applied to the resolved service price, not the provider defaults.

  2. continues serving a free service even when an existing paid channel is exhausted — directly tests the regression. The mock sets getCumulativeSpend: () => 1_000_000n (= reserve max), which would previously have triggered isAtExactSpendLimit and fired settleSession. The assertion that settleSession is not called confirms the fix works end-to-end.

Observation: The exhausted-channel test uses makeSpmMock defaults (hasSession: () => true, getChannelByPeer: () => ({ sessionId: ... })), meaning it has an active paid session AND is exhausted. This means spmAuthorized = true, so the first 402 block is already bypassed, and the test is specifically exercising the budget check guard. This is the right setup for isolating that scenario.


No blocking issues. This is ready to merge.

@kotevcode kotevcode merged commit 547dbbd into main May 26, 2026
3 checks passed
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