Skip to content

Count Stripe calls and finish activity backfill once#1881

Merged
stefan-burke merged 5 commits into
mainfrom
work/maintenance-budget-followups
Jul 22, 2026
Merged

Count Stripe calls and finish activity backfill once#1881
stefan-burke merged 5 commits into
mainfrom
work/maintenance-budget-followups

Conversation

@stefan-burke

@stefan-burke stefan-burke commented Jul 21, 2026

Copy link
Copy Markdown
Member

What changed

  • count every Stripe transport attempt, including HTTP retries and network failures, against the shared external subrequest budget
  • preserve completed activity-log backfills with the maintenance task checkpoint instead of rescanning the full log on every wake
  • migrate sites whose old completed marker deliberately left no maintenance row to the new completed checkpoint
  • bump the database update marker so existing sites run that data migration instead of only recording it
  • preserve unfinished tasks on new sites and keep disabled-task, exact-full-batch, and migration-rerun behavior directly covered

Why

These changes address the review findings added to #1862 after it merged. They close the remaining budget blind spot and remove repeated unindexed activity-log scans, including the first scan that old completed sites would otherwise perform after upgrading.

Verification

  • nix develop -c deno task precommit
  • Stripe request mutation: 134/134
  • activity backfill mutation: 14/14
  • maintenance registry mutation: 27/27
  • completed-checkpoint migration mutation: 4/4
  • schema version mutation: 5/5

Summary by CodeRabbit

  • New Features
    • Added resumable, checkpoint-based activity-log backfill with an explicit completion marker and follow-up scheduling until the batch is drained.
    • Added a schema migration to preserve completed activity-log backfills as complete (avoiding rescans).
  • Bug Fixes
    • Stripe external subrequests are now counted per retry attempt against the shared external budget.
    • Stripe operations are rejected before external network activity when external allowance is exhausted.
  • Maintenance
    • Disabled maintenance tasks are removed from scheduling without running.
  • Tests
    • Expanded coverage for activity-log backfill checkpointing, runner disabling behavior, and Stripe retry/budget scenarios.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The PR adds external subrequest accounting to Stripe retries and changes activity-log backfill to return progress counts and use completion checkpoints for resumable maintenance execution. It also adds a migration preserving completed backfill state and expands related tests.

Changes

Activity-log backfill checkpointing

Layer / File(s) Summary
Backfill result contract
src/shared/db/activity-log-backfill.ts, test/shared/db/activity-log-backfill.test.ts
The backfill exports a completion marker, removes legacy-presence detection, returns converted-row counts, and verifies completed tasks do not rescan activity_log.
Maintenance checkpoint flow
src/shared/maintenance/registry.ts, test/shared/maintenance/registry.test.ts, test/shared/maintenance/runner.test.ts
The task remains enabled, skips completed checkpoints, records completion after the final batch, requests follow-up execution when needed, and removes disabled tasks without running them.
Completion checkpoint migration
src/shared/db/migrations/*, test/shared/db/migrations/*, test/integration/migration-restore-verify.test.ts
A migration restores missing backfill tasks with a completed checkpoint while preserving unfinished rows, and migration ordering, metadata, and restore guards are updated.

Stripe subrequest budgeting

Layer / File(s) Summary
Stripe retry accounting
src/shared/stripe/request.ts, test/shared/stripe/request.test.ts, scripts/mutation/equivalent-mutants.txt
Stripe attempts count external subrequests before fetch execution; tests cover retries, rejected transport attempts, zero-allowance blocking, and updated equivalent-mutant coordinates.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant MaintenanceRunner
  participant MaintenanceRegistry
  participant ActivityLogBackfill
  MaintenanceRunner->>MaintenanceRegistry: run activity_log_backfill task
  MaintenanceRegistry->>ActivityLogBackfill: runActivityLogBackfill(publicKey)
  ActivityLogBackfill-->>MaintenanceRegistry: converted row count
  MaintenanceRegistry->>MaintenanceRunner: set complete checkpoint or request follow-up
Loading
sequenceDiagram
  participant StripeCaller
  participant StripeRequest
  participant StripeAPI
  StripeCaller->>StripeRequest: execute Stripe operation
  StripeRequest->>StripeRequest: count external subrequest
  StripeRequest->>StripeAPI: fetch request
  StripeAPI-->>StripeRequest: response or retryable failure
  StripeRequest-->>StripeCaller: result or blocked-operation error
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the two main changes: counting Stripe calls and completing activity-log backfill once.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch work/maintenance-budget-followups

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/shared/stripe/request.test.ts`:
- Around line 197-214: Extend the Stripe retry tests around recordingCheckout
and config.fetch to cover a network failure: make the first fetch attempt reject
and the retry succeed, then assert getSubrequestUsage() reports database 0,
external 2, and total 2 within runWithSubrequestBudget. Keep the existing HTTP
500 retry coverage unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 83b7ffb8-1598-4531-9494-c6a45df9822e

📥 Commits

Reviewing files that changed from the base of the PR and between 22490f6 and eb39445.

📒 Files selected for processing (8)
  • scripts/mutation/equivalent-mutants.txt
  • src/shared/db/activity-log-backfill.ts
  • src/shared/maintenance/registry.ts
  • src/shared/stripe/request.ts
  • test/shared/db/activity-log-backfill.test.ts
  • test/shared/maintenance/registry.test.ts
  • test/shared/maintenance/runner.test.ts
  • test/shared/stripe/request.test.ts

Comment thread test/shared/stripe/request.test.ts

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: eb39445345

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/shared/maintenance/registry.ts

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/shared/db/migrations/2026-07-21_activity_backfill_complete.test.ts`:
- Around line 34-62: Add a test alongside the existing migration cases that
seeds an already completed activity_log_backfill task, calls runMigration(), and
asserts activityTask() returns exactly one unchanged row with
ACTIVITY_LOG_BACKFILL_COMPLETE and its original next_run_at value. Use the
existing task setup and expectation patterns.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e3e89301-ccb7-4edf-a43e-98f0fba96255

📥 Commits

Reviewing files that changed from the base of the PR and between eb39445 and 15ae278.

📒 Files selected for processing (7)
  • src/shared/db/migrations/2026-07-21_activity_backfill_complete.ts
  • src/shared/db/migrations/registry.ts
  • test/integration/migration-restore-verify.test.ts
  • test/shared/db/migrations/2026-07-21_activity_backfill_complete.test.ts
  • test/shared/db/migrations/registry.test.ts
  • test/shared/db/migrations/schema/version/guard.test.ts
  • test/shared/stripe/request.test.ts

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1891fa3838

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/shared/db/migrations/registry.ts
@stefan-burke
stefan-burke added this pull request to the merge queue Jul 22, 2026
Merged via the queue into main with commit d487b37 Jul 22, 2026
5 checks passed
@stefan-burke
stefan-burke deleted the work/maintenance-budget-followups branch July 22, 2026 10:01
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