Skip to content

test(api): HTTP integration tests for pending-operations endpoints (#81) - #178

Merged
thomasluizon merged 2 commits into
mainfrom
issue-81
Jun 5, 2026
Merged

test(api): HTTP integration tests for pending-operations endpoints (#81)#178
thomasluizon merged 2 commits into
mainfrom
issue-81

Conversation

@thomasluizon

Copy link
Copy Markdown
Owner

Summary

Adds PendingOperationsControllerTests driving the full agent confirmation flow over real HTTP against Postgres (orbit_test): POST /api/ai/pending-operations/{id}/confirm -> /step-up -> /step-up/verify -> /execute, anchored on the api_keys.manage (StepUp) capability so every gate is exercised.

What it covers

  • confirm — 200 + token persisted (ConfirmedAtUtc/ConfirmationTokenHash in DB); unknown id -> 404.
  • step-up — challenge issued (200, code captured via the email fake); immediate second request -> 400 service cooldown (60s).
  • step-up/verify — valid code sets StepUpSatisfiedAtUtc; invalid code -> 400; repeated verifies trip the auth rate limiter (429).
  • execute — confirm-only (no step-up) -> 200 body Status=PendingConfirmation, PolicyReason=step_up_required; full confirm+step-up path -> Succeeded (creates a real API key); unknown id -> 404.

Seam added

IntegrationTestWebApplicationFactory registers a capturing IEmailService (CapturingEmailService) via ConfigureTestServices, exposing LastVerificationCode so the emailed-only step-up code is readable. Mirrors TestEmailService in AgentStepUpServiceTests. The existing JWT UseSetting is preserved (added to, not clobbered).

Notes

Refs thomasluizon/orbit-ui-mobile#81

🤖 Generated with Claude Code

Comment thread tests/Orbit.IntegrationTests/PendingOperationsControllerTests.cs

@claude claude 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.

One correctness bug: StepUpVerify_ExceedsAuthRateLimit_ReturnsTooManyRequests exhausts _userId's auth rate-limit quota (5 req/min, partitioned per-user in DistributedRateLimitAttribute), so Execute_AfterConfirmAndStepUp_Succeeds — which calls the same verify endpoint as the same user immediately after — will receive 429 instead of 200 whenever both tests land in the same clock-minute window. Detailed on the inline comment. Fix: give the rate-limit test its own user account so the exhausted bucket is isolated.

@claude claude 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.

Solid integration test suite for the pending-operations flow. The full confirm → step-up → step-up/verify → execute path is exercised end-to-end against a real DB, all gates (cooldown, invalid code, missing step-up, rate limit) are covered, and the rate-limit case is correctly isolated to its own user to prevent bleed across tests. The CapturingEmailService seam is cleanly wired via ConfigureTestServices / RemoveAll, and the JSONB-normalization trick in NormalizeToJsonb correctly handles the key-reordering that would otherwise break fingerprint matching at execute time. No correctness, security, or contract issues found.

Base automatically changed from chore/fix-integration-jwt-secret to main June 4, 2026 23:54
thomasluizon and others added 2 commits June 4, 2026 21:03
Add PendingOperationsControllerTests driving the full agent confirmation
flow over real HTTP against Postgres: confirm -> step-up -> step-up/verify
-> execute, anchored on the api_keys.manage (StepUp) capability.

Adds a capturing IEmailService fake (CapturingEmailService) to the test
factory via ConfigureTestServices so the step-up code (emailed only) is
readable by the test. Asserts confirmation-token persistence, step-up
challenge issuance + 60s service cooldown, valid/invalid code verification,
the auth rate limiter on repeated verifies, execute-time step_up_required
enforcement, and the full happy path creating a real API key.

Refs thomasluizon/orbit-ui-mobile#81

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The auth rate-limit (5/min, partitioned by user) is shared across every
step-up and verify call. StepUpVerify_ExceedsAuthRateLimit deliberately
exhausts it, and as a sibling of the other pending-ops tests on the same
class-wide account that bled 429s into them whenever they shared a
clock-minute. Move it into a dedicated nested RateLimitTests class with
its own IAsyncLifetime and its own registered account so the exhausted
bucket is isolated. Shared seeding helpers become static and parameterized.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@thomasluizon
thomasluizon merged commit edc4a83 into main Jun 5, 2026
3 checks passed
@thomasluizon
thomasluizon deleted the issue-81 branch June 5, 2026 00:04
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