Skip to content

Harden finance integration: auth-safe live connectors, validated query inputs, normalized error taxonomy, and pluggable shared cache - #19

Merged
charles2ke merged 4 commits into
mainfrom
copilot/implement-recommendation-set-1
Sep 5, 2026
Merged

Harden finance integration: auth-safe live connectors, validated query inputs, normalized error taxonomy, and pluggable shared cache#19
charles2ke merged 4 commits into
mainfrom
copilot/implement-recommendation-set-1

Conversation

Copilot AI commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

This PR closes remaining recommendation-set #1 gaps in the finance surface by hardening live connector behavior, enforcing finance query input validity, normalizing API-safe error categorization, and extending cache pluggability for shared backends. Existing query contracts remain backward-compatible while making failure behavior more predictable and operationally safer.

  • 1) Live authenticated upstream connectors

    • Enforced safe behavior when a non-mock:// endpoint is configured without credentials.
    • Added a misconfigured-auth connector path that:
      • reports degraded in readiness,
      • returns auth-classified upstream failures,
      • avoids sensitive error leakage.
    • Preserved mock/live auto-selection by endpoint.
  • 2) Finance query enhancements (validation + predictable defaults)

    • Added resolver-boundary validation for finance args:
      • ISO date validity (from, to),
      • range ordering (from <= to),
      • non-negative pagination (limit, offset),
      • bounded taxYear.
    • Invalid inputs now fail fast with BAD_USER_INPUT and structured validation details.
    • Existing pagination metadata/defaulting behavior is preserved.
  • 3) Observability and error taxonomy

    • Standardized upstream error messages into non-sensitive category-based API responses.
    • Added resolver-boundary structured logging and resolver metrics timing/counters.
    • Mapped unexpected resolver failures to API-safe internal GraphQL errors (INTERNAL_SERVER_ERROR, category: internal) to avoid leaking internals.
  • 4) Optional persistent/shared cache strategy

    • Extended cache abstraction with shared store mode:
      • FINANCE_CACHE_STORE=shared
      • FINANCE_CACHE_SHARED_MODULE=<module path>
    • Added runtime module contract (createSharedCacheStore) with capability checks.
    • Implemented explicit fallback to in-memory cache when shared provider module is missing/invalid, with warning logs.
  • Checklist mapped to goals 1–4

    • Goal 1: Live authenticated connector safety and readiness degradation for missing credentials
    • Goal 2: Finance input validation at resolver boundaries with predictable error semantics
    • Goal 3: Structured resolver observability + normalized API-safe error categories
    • Goal 4: Pluggable shared cache provider support with safe memory fallback
  • Representative change

if (issues.length > 0) {
  throw new GraphQLError('Invalid finance query arguments.', {
    extensions: { code: 'BAD_USER_INPUT', category: 'validation', details: issues },
  });
}
  • Follow-up items
    • Consider schema-level typed constraints (custom scalars/enums) for finance filters to shift more validation to GraphQL parsing.
    • Add a first-party shared cache adapter package (e.g., Redis-backed) implementing the new provider contract.

Co-authored-by: charles2ke <6725706+charles2ke@users.noreply.github.com>
Copilot AI changed the title [WIP] Implement recommendation set #1 for GraphQL connectors and queries Harden finance integration: auth-safe live connectors, validated query inputs, normalized error taxonomy, and pluggable shared cache Sep 5, 2026
Copilot AI requested a review from charles2ke September 5, 2026 09:09
@charles2ke
charles2ke marked this pull request as ready for review September 5, 2026 09:12
@charles2ke
charles2ke requested a lite review from Copilot September 5, 2026 09:12

Copilot AI 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.

🟡 Changes recommended

Resolver date validation is currently bypassable/too permissive (empty strings and non-ISO formats), and README documentation is inconsistent with implemented cache-store values and GraphQL error taxonomy.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR hardens the finance integration by adding safer behavior for live (non-mock://) connectors, enforcing resolver-boundary input validation for finance queries, normalizing upstream error messages to non-sensitive taxonomy, and extending cache pluggability with an optional shared cache provider module.

Changes:

  • Added “auth misconfigured” live connector mode when endpoints are set without API keys, reporting degraded readiness and returning auth-classified upstream failures.
  • Wrapped finance resolvers with centralized argument validation + API-safe internal error mapping, plus basic resolver metrics/logging hooks.
  • Added FINANCE_CACHE_STORE=shared support via a runtime-loaded provider module with capability checks and in-memory fallback; updated docs/tests accordingly.
File summaries
File Description
test/resilience.test.js Adds tests for shared cache provider module loading and fallback behavior.
test/graphql.test.js Updates upstream error message assertions and adds tests for finance arg validation + internal error mapping.
test/finance.test.js Extends config parsing tests to include FINANCE_CACHE_SHARED_MODULE.
test/connectors.test.js Adds coverage for safe failure/readiness degradation when live endpoints lack credentials.
src/services/financeService.js Wires cacheSharedModule into cache store creation.
src/resolvers.js Introduces finance resolver wrapper for validation, metrics/logging, and safe internal error mapping.
src/observability/errors.js Normalizes upstream error messages by category to avoid leaking raw upstream details.
src/connectors/index.js Adds auth-misconfigured connector path and enforces missing-credential handling for live endpoints.
src/config/finance.js Adds cacheSharedModule configuration from FINANCE_CACHE_SHARED_MODULE.
src/cache/index.js Implements shared cache store mode via runtime module loading with validation and fallback.
README.md Documents shared cache provider configuration and finance validation/error behavior.
Review details

Suppressed comments (1)

README.md:282

  • The README claims GraphQL-safe categories include auth and upstream, but current resolver-boundary GraphQL errors only set extensions.category to validation (BAD_USER_INPUT) or internal (INTERNAL_SERVER_ERROR). Upstream auth/upstream failures are returned in the payload errors[] with uppercase categories.
- **API-safe taxonomy**: finance resolver input and internal failures are
  normalized to GraphQL-safe categories: `validation`, `auth`, `upstream`, and
  `internal`, with non-sensitive messages.
  • Files reviewed: 11/11 changed files
  • Comments generated: 3
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/resolvers.js
Comment thread README.md Outdated
Comment thread src/cache/index.js
Co-authored-by: charles2ke <6725706+charles2ke@users.noreply.github.com>
Co-authored-by: charles2ke <6725706+charles2ke@users.noreply.github.com>
@charles2ke
charles2ke merged commit 3951989 into main Sep 5, 2026
6 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.

3 participants