[1529] Improve verifier quorum and administrator transitions: bounded performance and operational visibility(closes #1529) - #1597
Merged
1nonlypiece merged 2 commits intoAug 30, 2026
Conversation
… performance and operational visibility Refs Disciplr-Org#1529 ## What changed ### src/routes/adminVerifiers.ts - Enforce MAX_PAGE_LIMIT=200 on GET / list; default reduced to 50. Response now includes pagination metadata (limit, offset, count, hasMore). - Add emitDiagnostic() helper: structured JSON events (action, requestId, actorUserId, targetUserId, latencyMs, outcome, errorCode, fromStatus, toStatus) emitted to stderr on every transition, with no secrets or PII. - Set X-Request-Id response header on all transition endpoints; echoes client-supplied header when present, otherwise generates a UUID. - Add inFlightTransitions Set to guard /approve and /suspend against duplicate concurrent mutations for the same userId (429 response). - Update reinstate, deactivate, reactivate handlers to emit diagnostics. ### src/routes/verifications.ts - Add Router import (was accidentally removed) and pagination to GET /: default limit=100, max limit=500, offset param, hasMore flag, pagination metadata in response. Prevents unbounded full-table scans. - Add bulkInFlight Set: rejects a second concurrent bulk submission from the same verifier with 400 to prevent duplicate evidence writes. - Add emitVerificationDiagnostic() helper for bulk action telemetry (latencyMs, outcome success/partial, count, no URLs or secrets logged). - Wrap bulk loop body in try/finally so bulkInFlight.delete() is always called even if an unexpected error escapes the per-item catch. ### src/services/verifiers.ts - listVerifierProfiles: clamp uncapped caller limit to MAX_VERIFIER_PROFILES_LIMIT=200; export the constant. - listVerifications: add ListVerificationsOptions (limit, offset) with MAX_VERIFICATIONS_LIMIT=500 cap; export both. - getMilestoneApprovalProgress: enforce invariants — approvalThreshold must be a positive integer; totalVerifiers when provided must be a positive integer and ≥ approvalThreshold. ### src/tests/adminVerifiers.quorum.test.ts (new) - Pagination bounds (default, cap, hasMore, negative/NaN limit). - Permission matrix (USER, VERIFIER, ADMIN, unauthenticated). - Full transition matrix (approve, suspend, deactivate, reactivate). - Reinstate: already-active no-op, suspended→approved, suspended→pending, deactivated→approved, not-found 404. - Concurrent in-flight guard (429, clears after first completes). - Diagnostic log structure (no secrets, includes required fields). - X-Request-Id echoing. - userId sanitization boundary (whitespace, >128 chars, exactly 128). ### src/tests/verifications.quorum.test.ts (new) - Paginated admin list (defaults, cap, hasMore, non-numeric limit, VERIFIER 403 on GET). - Bulk in-flight guard (same verifier → 400, sequential ok, different verifiers not blocked). - Bulk diagnostic emission (success outcome, partial outcome). - Bulk boundary invariants (>100 items → 400, =100 accepted, empty → 400, non-array → 400). ### src/tests/verifiers.service.quorum.test.ts (new) - listVerifierProfiles: MAX_VERIFIER_PROFILES_LIMIT=200, clamping, default, negative/Infinity limit, negative offset. - listVerifications: MAX_VERIFICATIONS_LIMIT=500, clamping, default, offset, whereIn with/without targetIds. - canTransition: full valid + invalid edge coverage (16 cases). - getMilestoneApprovalProgress: invariant rejection (threshold=0, negative, float, totalVerifiers<threshold, totalVerifiers=0/negative), valid cases. ## Design tradeoffs - inFlightTransitions and bulkInFlight are process-local Sets. In a multi-replica deployment these guards only prevent intra-process duplicates; a Redis SETNX lock would be needed for cross-process protection. The limitation is documented in code comments. - Diagnostics use console.error (stderr) for structured JSON so they flow to existing log aggregators without adding a new dependency. They omit evidence URLs and raw user-controlled strings to avoid log injection. - Pagination uses offset/limit (not cursor) to match the existing pattern already established on other admin list endpoints in this codebase. ## Limitations - node_modules not present in this environment; tests validated by structural review against the Jest ESM mock pattern used in the existing test suite.
|
@danielchukuma-dev Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1529
Summary
This PR addresses all acceptance criteria for #1529: bounded rendering/polling/network/memory costs and observable degraded behavior for the verifier quorum and administrator transitions state machine anchored at
src/routes/verifications.tsandsrc/routes/adminVerifiers.ts.Changes
src/routes/adminVerifiers.tsGET /enforcesMAX_PAGE_LIMIT=200; default reduced to 50. Response includespaginationmetadata (limit,offset,count,hasMore).emitDiagnostic()emits structured JSON events on every status transition — fields:action,requestId,actorUserId,targetUserId,latencyMs,outcome,errorCode,fromStatus,toStatus. No secrets or PII are included.X-Request-Idheader: All transition responses set this header; echoes the client-supplied value when present, otherwise generates a UUID.inFlightTransitionsSet prevents duplicate concurrent mutations for the sameuserIdon/approveand/suspend, returning429on collision. Limitation documented in code: process-local only; cross-replica protection requires a distributed lock (e.g. RedisSET NX PX).src/routes/verifications.tsGET /gainslimit(default 100, max 500) andoffsetparams pluspaginationmetadata in the response. Prevents unbounded full-table scans.bulkInFlightSet rejects a second concurrent bulk submission from the same verifier with400, preventing duplicate evidence records or DB amplification during rapid reconnects. Different verifiers are not blocked by each other.emitVerificationDiagnostic()emitslatencyMs,outcome(success/partial), andcountafter every bulk call. Evidence URLs and raw user strings are explicitly excluded.try/finallysobulkInFlight.delete()always runs even if an unexpected error escapes the per-item catch.src/services/verifiers.tslistVerifierProfiles: Clamps caller-suppliedlimittoMAX_VERIFIER_PROFILES_LIMIT=200(exported). Default changed from 100 → 50.listVerifications: Now acceptsListVerificationsOptions(limit,offset) withMAX_VERIFICATIONS_LIMIT=500cap (exported). Signature is backward-compatible.getMilestoneApprovalProgressinvariants: Throws with a descriptive message whenapprovalThreshold < 1,approvalThresholdis non-integer,totalVerifiers < 1, ortotalVerifiers < approvalThreshold— making the quorum configuration contract explicit.Tests
Three new focused test files following the existing Jest ESM (
jest.unstable_mockModule) pattern:src/tests/adminVerifiers.quorum.test.tssrc/tests/verifications.quorum.test.tssrc/tests/verifiers.service.quorum.test.tslistVerifierProfilesclamp,listVerificationspagination,canTransitionmatrix (16 cases),getMilestoneApprovalProgressinvariant rejectionValidation commands
Design tradeoffs
inFlightTransitionsandbulkInFlightareSets — zero-dependency, zero-latency, but process-local. For multi-replica deployments a RedisSETNXpattern is the production path. Documented inline.console.errorto stderr: Flows to existing log aggregators without adding a new runtime dependency. Structured JSON makes it grep-able and machine-parseable.Acceptance criteria checklist
Limitations
inFlightTransitionsandbulkInFlightare process-local — noted as a known limitation in code comments.