From 89ca60c0171c66bc669121b15969028a930ac683 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 5 Sep 2026 09:02:28 +0000 Subject: [PATCH 1/4] Initial plan From 579eef3b2a90bc626ac8fb6c55accc8df65e8e17 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 5 Sep 2026 09:07:55 +0000 Subject: [PATCH 2/4] Add finance input validation and shared cache provider fallback Co-authored-by: charles2ke <6725706+charles2ke@users.noreply.github.com> --- README.md | 15 +++++++++- src/cache/index.js | 28 ++++++++++++++++++- src/config/finance.js | 1 + src/connectors/index.js | 27 ++++++++++++++++++ src/observability/errors.js | 14 ++++++++-- src/resolvers.js | 51 ++++++++++++++++++++++++++++++++-- src/services/financeService.js | 2 +- test/connectors.test.js | 15 +++++++++- test/finance.test.js | 8 +++++- test/graphql.test.js | 33 +++++++++++++++++++++- test/resilience.test.js | 41 ++++++++++++++++++++++++++- 11 files changed, 223 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 4ffda5d..995c862 100644 --- a/README.md +++ b/README.md @@ -148,6 +148,7 @@ The running server loads connector settings from the environment via | `FINANCE_CACHE_TTL_MS` | Resolver cache TTL | `1000` | | `FINANCE_CACHE_STORE` | Cache strategy: `memory` or `file` (persistent) | `memory` | | `FINANCE_CACHE_FILE` | Cache file used when `FINANCE_CACHE_STORE=file` | `.cache/finance-cache.json` | +| `FINANCE_CACHE_SHARED_MODULE` | Optional shared cache provider module path used when `FINANCE_CACHE_STORE=shared` | empty | | `FINANCE_HTTP_TIMEOUT_MS` | Per-request upstream timeout | `5000` | | `FINANCE_HTTP_MAX_RETRIES` | Retries for timeouts, 429s, and 5xx responses | `2` | | `FINANCE_DEFAULT_PAGE_SIZE` | Default page size when `limit` is omitted | `25` | @@ -158,7 +159,9 @@ Each connector endpoint that is **not** a `mock://` URL is served by the production HTTP client in `src/connectors/httpClient.js`, which adds bearer authentication, request timeouts, bounded retries with exponential backoff, and per-call metrics. Mock adapters remain the default so the service still runs -from a clean checkout. +from a clean checkout. Live endpoints require the corresponding `*_API_KEY`; +when credentials are missing, connectors fail safely with a non-sensitive auth +error and readiness reports `degraded`. Do not commit real credentials. Production connectors should read credentials from environment variables or a secret manager and keep the same method names as @@ -245,6 +248,8 @@ Finance queries accept optional filters and offset pagination: Every finance payload includes `pageInfo { totalCount limit offset hasNextPage hasPreviousPage }`. Requested limits are clamped to `FINANCE_MAX_PAGE_SIZE`. +Invalid date ranges (`from > to`) and malformed date/pagination inputs are +rejected with `BAD_USER_INPUT` and `extensions.category = "validation"`. ```graphql query RecentSells { @@ -272,6 +277,9 @@ query RecentSells { `retryable`. Those fields are returned on every payload's `errors { source code category status retryable message }`, so a partial response still explains what failed and whether retrying helps. +- **API-safe taxonomy**: finance resolver input and internal failures are + normalized to GraphQL-safe categories: `validation`, `auth`, `upstream`, and + `internal`, with non-sensitive messages. - **Health**: `GET /health` is a liveness probe; `GET /ready` calls each connector's health check and returns `503` when any upstream is degraded. @@ -283,6 +291,11 @@ Upstream reads go through a TTL cache selected by `FINANCE_CACHE_STORE` - `memory` (default): in-process, fastest, cleared on restart. - `file`: the same TTL semantics mirrored to `FINANCE_CACHE_FILE`, so a restarted process serves warm upstream data instead of refetching everything. +- `shared`: optional provider loaded from `FINANCE_CACHE_SHARED_MODULE`. The + module must export `createSharedCacheStore()` returning a store with + `get(key)`, `set(key, value, ttlMs)`, and `clear()` methods (Redis-like + adapters can implement this contract). If loading fails, the service logs a + warning and falls back to `memory`. Concurrent resolvers asking for the same key share one in-flight request, and payloads containing upstream errors are never cached so a transient outage is diff --git a/src/cache/index.js b/src/cache/index.js index b2cb55b..c7ff05c 100644 --- a/src/cache/index.js +++ b/src/cache/index.js @@ -1,9 +1,12 @@ import { readFileSync } from 'node:fs'; import { mkdir, rename, writeFile } from 'node:fs/promises'; +import { createRequire } from 'node:module'; import { dirname } from 'node:path'; import { logger as defaultLogger } from '../observability/logger.js'; +const require = createRequire(import.meta.url); + /** Default in-process cache: fastest option, lost on restart. */ export function createMemoryCacheStore() { const entries = new Map(); @@ -115,8 +118,31 @@ export function createFileCacheStore({ file, logger = defaultLogger } = {}) { } /** Selects a cache store from configuration (`memory` by default). */ -export function createCacheStore({ store = 'memory', file, logger = defaultLogger } = {}) { +export function createCacheStore({ store = 'memory', file, sharedModule, logger = defaultLogger } = {}) { if (store === 'file') return createFileCacheStore({ file, logger }); + if (store === 'shared') { + if (sharedModule) { + try { + const loaded = require(sharedModule); + const factory = loaded.createSharedCacheStore ?? loaded.default; + if (typeof factory === 'function') { + const sharedStore = factory({ logger }); + if (sharedStore && typeof sharedStore.get === 'function' && typeof sharedStore.set === 'function' && typeof sharedStore.clear === 'function') { + return { kind: sharedStore.kind ?? 'shared', ...sharedStore }; + } + } + logger.warn('shared cache module is invalid, falling back to memory', { sharedModule }); + } catch (error) { + logger.warn('shared cache module could not be loaded, falling back to memory', { + sharedModule, + error: error?.message ?? String(error), + }); + } + } else { + logger.warn('shared cache store requested without module, falling back to memory'); + } + return createMemoryCacheStore(); + } if (store !== 'memory') { logger.warn('unknown cache store, falling back to memory', { store }); } diff --git a/src/config/finance.js b/src/config/finance.js index d109745..4942f99 100644 --- a/src/config/finance.js +++ b/src/config/finance.js @@ -31,6 +31,7 @@ export function loadFinanceConfig(env = process.env) { // short-lived workers can reuse warm upstream data. cacheStore: env.FINANCE_CACHE_STORE ?? 'memory', cacheFile: env.FINANCE_CACHE_FILE ?? '.cache/finance-cache.json', + cacheSharedModule: env.FINANCE_CACHE_SHARED_MODULE ?? '', defaultPageSize: Number(env.FINANCE_DEFAULT_PAGE_SIZE ?? 25), maxPageSize: Number(env.FINANCE_MAX_PAGE_SIZE ?? 100), }; diff --git a/src/connectors/index.js b/src/connectors/index.js index b5d12e1..5b23681 100644 --- a/src/connectors/index.js +++ b/src/connectors/index.js @@ -7,6 +7,7 @@ import { createPortfolioWatcherConnector } from './portfolio-watcher/mockConnect import { createPortfolioWatcherHttpConnector } from './portfolio-watcher/httpConnector.js'; import { createTaxBreakConnector } from './tax-break/mockConnector.js'; import { createTaxBreakHttpConnector } from './tax-break/httpConnector.js'; +import { UpstreamHttpError } from './httpClient.js'; /** Mock endpoints keep the service runnable from a clean checkout. */ export function isMockEndpoint(endpoint = '') { @@ -19,6 +20,27 @@ const FACTORIES = { taxBreak: { mock: createTaxBreakConnector, http: createTaxBreakHttpConnector }, }; +const CONNECTOR_METHODS = { + openTrading: ['listAccounts', 'listTrades', 'listOrders'], + portfolioWatcher: ['listPositions', 'listPerformanceSnapshots'], + taxBreak: ['mapTradesToTaxEvents', 'estimateTax'], +}; + +function createAuthMisconfiguredConnector(name, endpoint) { + const source = name === 'portfolioWatcher' ? 'Portfolio-Watcher' : name === 'taxBreak' ? 'tax-break' : 'OpenTrading'; + const buildError = () => + new UpstreamHttpError(source, `${source} connector is missing API credentials`, { kind: 'auth', retryable: false }); + + return { + source, + endpoint, + async health() { + return { source, status: 'degraded', endpoint, error: 'missing API credentials' }; + }, + ...Object.fromEntries(CONNECTOR_METHODS[name].map((method) => [method, async () => Promise.reject(buildError())])), + }; +} + /** * Builds one connector per upstream domain, choosing the production HTTP client * whenever a real endpoint is configured and falling back to the mock adapter @@ -39,6 +61,11 @@ export function createConnectors({ // runs and local demos are not noisy. logger[mode === 'http' ? 'info' : 'debug']('finance connector configured', { connector: name, mode, endpoint: settings.endpoint }); + if (mode === 'http' && !settings.apiKey) { + logger.error('finance connector missing credentials', { connector: name, endpoint: settings.endpoint }); + return [name, createAuthMisconfiguredConnector(name, settings.endpoint)]; + } + return [ name, mode === 'mock' diff --git a/src/observability/errors.js b/src/observability/errors.js index 0929c7e..d85b5e4 100644 --- a/src/observability/errors.js +++ b/src/observability/errors.js @@ -6,6 +6,7 @@ * generic outages, and callers can decide whether a retry is worthwhile. */ export const ERROR_CATEGORIES = { + VALIDATION: 'VALIDATION', AUTH: 'AUTH', RATE_LIMIT: 'RATE_LIMIT', TIMEOUT: 'TIMEOUT', @@ -25,6 +26,7 @@ function categoryFromStatus(status) { } function categoryFromError(error) { + if (error?.kind === 'auth') return ERROR_CATEGORIES.AUTH; if (error?.kind === 'timeout' || error?.name === 'AbortError') return ERROR_CATEGORIES.TIMEOUT; if (error?.kind === 'network') return ERROR_CATEGORIES.NETWORK; return ERROR_CATEGORIES.UNKNOWN; @@ -43,9 +45,17 @@ const RETRYABLE = new Set([ * the schema. */ export function classifyUpstreamError(source, error) { - const message = error instanceof Error ? error.message : String(error); const status = Number.isInteger(error?.status) ? error.status : null; const category = status === null ? categoryFromError(error) : categoryFromStatus(status); + const messageByCategory = { + [ERROR_CATEGORIES.AUTH]: `${source} connector authentication failed`, + [ERROR_CATEGORIES.RATE_LIMIT]: `${source} connector rate limit exceeded`, + [ERROR_CATEGORIES.TIMEOUT]: `${source} connector request timed out`, + [ERROR_CATEGORIES.NETWORK]: `${source} connector network error`, + [ERROR_CATEGORIES.UPSTREAM_CLIENT_ERROR]: `${source} connector request rejected`, + [ERROR_CATEGORIES.UPSTREAM_SERVER_ERROR]: `${source} connector is unavailable`, + [ERROR_CATEGORIES.UNKNOWN]: `${source} connector request failed`, + }; return { source, @@ -53,6 +63,6 @@ export function classifyUpstreamError(source, error) { category, status, retryable: typeof error?.retryable === 'boolean' ? error.retryable : RETRYABLE.has(category), - message: `${source} connector failed: ${message}`, + message: status === null ? messageByCategory[category] : `${messageByCategory[category]} (HTTP ${status})`, }; } diff --git a/src/resolvers.js b/src/resolvers.js index 4ea6abe..f523fed 100644 --- a/src/resolvers.js +++ b/src/resolvers.js @@ -1,5 +1,49 @@ import { GraphQLError } from 'graphql'; +function isIsoDate(value) { + return Number.isFinite(new Date(value).getTime()); +} + +function validateFinanceArgs(args, { requireTaxYear = false } = {}) { + const issues = []; + if (args.from && !isIsoDate(args.from)) issues.push('from must be a valid ISO-8601 date'); + if (args.to && !isIsoDate(args.to)) issues.push('to must be a valid ISO-8601 date'); + if (args.from && args.to && isIsoDate(args.from) && isIsoDate(args.to) && new Date(args.from).getTime() > new Date(args.to).getTime()) { + issues.push('from must be earlier than or equal to to'); + } + if (args.limit !== undefined && args.limit !== null && args.limit < 0) issues.push('limit must be greater than or equal to 0'); + if (args.offset !== undefined && args.offset !== null && args.offset < 0) issues.push('offset must be greater than or equal to 0'); + if (requireTaxYear && (args.taxYear < 1900 || args.taxYear > 9999)) issues.push('taxYear must be between 1900 and 9999'); + return issues; +} + +async function runFinanceResolver(name, args, context, resolve, options = {}) { + const issues = validateFinanceArgs(args, options); + if (issues.length > 0) { + context.metrics?.increment?.('finance_resolver_total', { resolver: name, outcome: 'validation_error' }); + throw new GraphQLError('Invalid finance query arguments.', { + extensions: { code: 'BAD_USER_INPUT', category: 'validation', details: issues }, + }); + } + + const execute = () => + context.metrics?.time + ? context.metrics.time('finance_resolver', { resolver: name }, resolve) + : resolve(); + + try { + const result = await execute(); + context.logger?.info?.('finance resolver completed', { resolver: name, outcome: 'success' }); + return result; + } catch (error) { + if (error instanceof GraphQLError) throw error; + context.logger?.error?.('finance resolver failed', { resolver: name, category: 'internal', error: error?.message ?? String(error) }); + throw new GraphQLError('Finance query failed. Please retry later.', { + extensions: { code: 'INTERNAL_SERVER_ERROR', category: 'internal' }, + }); + } +} + /** * Resolvers read and write through the store provided on the GraphQL context, * which keeps them independent from the concrete storage implementation. @@ -10,9 +54,10 @@ export const resolvers = { user: (_parent, { id }, { store }) => store.getUser(id), posts: (_parent, _args, { store }) => store.listPosts(), post: (_parent, { id }, { store }) => store.getPost(id), - portfolioOverview: (_parent, args, { finance }) => finance.portfolioOverview(args), - tradeHistory: (_parent, args, { finance }) => finance.tradeHistory(args), - taxEstimate: (_parent, args, { finance }) => finance.taxEstimate(args), + portfolioOverview: (_parent, args, context) => runFinanceResolver('portfolioOverview', args, context, () => context.finance.portfolioOverview(args)), + tradeHistory: (_parent, args, context) => runFinanceResolver('tradeHistory', args, context, () => context.finance.tradeHistory(args)), + taxEstimate: (_parent, args, context) => + runFinanceResolver('taxEstimate', args, context, () => context.finance.taxEstimate(args), { requireTaxYear: true }), }, Mutation: { diff --git a/src/services/financeService.js b/src/services/financeService.js index 073c8c7..cbe4641 100644 --- a/src/services/financeService.js +++ b/src/services/financeService.js @@ -30,7 +30,7 @@ export function createFinanceService({ } = {}) { const upstreams = connectors ?? createConnectors({ config, logger, metrics }); const pageLimits = { defaultLimit: config.defaultPageSize ?? 25, maxLimit: config.maxPageSize ?? 100 }; - const cache = cacheStore ?? createCacheStore({ store: config.cacheStore, file: config.cacheFile, logger }); + const cache = cacheStore ?? createCacheStore({ store: config.cacheStore, file: config.cacheFile, sharedModule: config.cacheSharedModule, logger }); // Concurrent resolvers asking for the same upstream data share a single // in-flight request instead of fanning out duplicate connector calls. const inFlight = new Map(); diff --git a/test/connectors.test.js b/test/connectors.test.js index 10f973e..dd9f0c1 100644 --- a/test/connectors.test.js +++ b/test/connectors.test.js @@ -124,7 +124,7 @@ describe('connector selection', () => { assert.equal((await mocks.openTrading.health()).endpoint, 'mock://opentrading'); const live = createConnectors({ - config: loadFinanceConfig({ OPENTRADING_ENDPOINT: 'https://trading.example' }), + config: loadFinanceConfig({ OPENTRADING_ENDPOINT: 'https://trading.example', OPENTRADING_API_KEY: 'key' }), logger: silentLogger, metrics: createMetrics(), fetchImpl: async () => jsonResponse({ accounts: [] }), @@ -134,6 +134,19 @@ describe('connector selection', () => { assert.equal((await live.portfolioWatcher.health()).endpoint, 'mock://portfolio-watcher'); }); + it('fails safely when live endpoints are configured without credentials', async () => { + const live = createConnectors({ + config: loadFinanceConfig({ OPENTRADING_ENDPOINT: 'https://trading.example' }), + logger: silentLogger, + metrics: createMetrics(), + }); + + await assert.rejects(() => live.openTrading.listAccounts(), /missing API credentials/); + const health = await live.openTrading.health(); + assert.equal(health.status, 'degraded'); + assert.equal(health.error, 'missing API credentials'); + }); + it('reads timeout and retry settings from the environment', () => { const config = loadFinanceConfig({ FINANCE_HTTP_TIMEOUT_MS: '250', FINANCE_HTTP_MAX_RETRIES: '4', FINANCE_MAX_PAGE_SIZE: '10' }); assert.equal(config.openTrading.timeoutMs, 250); diff --git a/test/finance.test.js b/test/finance.test.js index d24e9b1..ed9733c 100644 --- a/test/finance.test.js +++ b/test/finance.test.js @@ -100,10 +100,16 @@ describe('finance service configuration and batching', () => { const defaults = loadFinanceConfig({}); assert.equal(defaults.openTrading.endpoint, 'mock://opentrading'); assert.equal(defaults.cacheTtlMs, 1000); + assert.equal(defaults.cacheSharedModule, ''); - const configured = loadFinanceConfig({ OPENTRADING_ENDPOINT: 'https://trading.example', FINANCE_CACHE_TTL_MS: '5000' }); + const configured = loadFinanceConfig({ + OPENTRADING_ENDPOINT: 'https://trading.example', + FINANCE_CACHE_TTL_MS: '5000', + FINANCE_CACHE_SHARED_MODULE: '/tmp/shared-cache.cjs', + }); assert.equal(configured.openTrading.endpoint, 'https://trading.example'); assert.equal(configured.cacheTtlMs, 5000); + assert.equal(configured.cacheSharedModule, '/tmp/shared-cache.cjs'); }); it('shares a single upstream call between concurrent and cached requests', async () => { diff --git a/test/graphql.test.js b/test/graphql.test.js index eff7078..832d9c7 100644 --- a/test/graphql.test.js +++ b/test/graphql.test.js @@ -244,6 +244,37 @@ describe('GraphQL API', () => { assert.equal(result.data.portfolioOverview.errors[0].category, 'UNKNOWN'); assert.equal(result.data.portfolioOverview.errors[0].status, null); assert.equal(result.data.portfolioOverview.errors[0].retryable, false); - assert.match(result.data.portfolioOverview.errors[0].message, /positions endpoint timed out/); + assert.equal(result.data.portfolioOverview.errors[0].message, 'Portfolio-Watcher connector request failed'); + }); + + it('returns validation errors for invalid finance date range and pagination args', async () => { + const invalidDate = await execute('{ tradeHistory(from: "not-a-date") { trades { id } } }'); + assert.equal(invalidDate.errors.length, 1); + assert.equal(invalidDate.errors[0].extensions.code, 'BAD_USER_INPUT'); + assert.equal(invalidDate.errors[0].extensions.category, 'validation'); + + const invalidRange = await execute('{ tradeHistory(from: "2026-02-01T00:00:00.000Z", to: "2026-01-01T00:00:00.000Z") { trades { id } } }'); + assert.equal(invalidRange.errors.length, 1); + assert.equal(invalidRange.errors[0].extensions.code, 'BAD_USER_INPUT'); + + const invalidLimit = await execute('{ portfolioOverview(limit: -1) { pageInfo { totalCount } } }'); + assert.equal(invalidLimit.errors.length, 1); + assert.equal(invalidLimit.errors[0].extensions.code, 'BAD_USER_INPUT'); + }); + + it('maps unexpected finance resolver failures to api-safe internal errors', async () => { + finance = { + portfolioOverview: async () => { + throw new Error('database credentials leaked'); + }, + tradeHistory: async () => ({ trades: [], orders: [], taxEvents: [], pageInfo: { totalCount: 0, limit: 0, offset: 0, hasNextPage: false, hasPreviousPage: false }, errors: [] }), + taxEstimate: async () => ({ taxYear: 2026, currency: 'USD', totalProceeds: 0, totalCostBasis: 0, realizedGain: 0, estimatedTax: 0, taxRate: 0.22, events: [], pageInfo: { totalCount: 0, limit: 0, offset: 0, hasNextPage: false, hasPreviousPage: false }, errors: [] }), + }; + + const result = await execute('{ portfolioOverview { totalMarketValue } }'); + assert.equal(result.errors.length, 1); + assert.equal(result.errors[0].message, 'Finance query failed. Please retry later.'); + assert.equal(result.errors[0].extensions.code, 'INTERNAL_SERVER_ERROR'); + assert.equal(result.errors[0].extensions.category, 'internal'); }); }); diff --git a/test/resilience.test.js b/test/resilience.test.js index 1149941..3d96bb8 100644 --- a/test/resilience.test.js +++ b/test/resilience.test.js @@ -17,6 +17,32 @@ function tempCacheFile() { return join(mkdtempSync(join(tmpdir(), 'finance-cache-')), 'cache.json'); } +function tempSharedModule() { + const directory = mkdtempSync(join(tmpdir(), 'finance-shared-module-')); + const path = join(directory, 'shared-cache.cjs'); + writeFileSync( + path, + `module.exports.createSharedCacheStore = function createSharedCacheStore() { + const entries = new Map(); + return { + kind: 'shared', + get(key) { + const hit = entries.get(key); + if (!hit || hit.expiresAt <= Date.now()) return undefined; + return hit.value; + }, + set(key, value, ttlMs) { + entries.set(key, { value, expiresAt: Date.now() + ttlMs }); + }, + clear() { + entries.clear(); + } + }; + };` + ); + return path; +} + describe('upstream error classification', () => { it('maps HTTP statuses and transport failures to stable categories', () => { const cases = [ @@ -34,7 +60,7 @@ describe('upstream error classification', () => { assert.equal(classified.category, category); assert.equal(classified.code, code); assert.equal(classified.retryable, retryable); - assert.match(classified.message, /OpenTrading connector failed/); + assert.match(classified.message, /OpenTrading connector/); } }); @@ -105,6 +131,19 @@ describe('cache strategies', () => { assert.equal(createCacheStore({ store: 'redis', logger: silentLogger }).kind, 'memory'); }); + it('loads a shared cache provider module when configured', () => { + const modulePath = tempSharedModule(); + const store = createCacheStore({ store: 'shared', sharedModule: modulePath, logger: silentLogger }); + store.set('k', { value: 1 }, 60_000); + assert.equal(store.kind, 'shared'); + assert.deepEqual(store.get('k'), { value: 1 }); + }); + + it('falls back to memory when shared cache provider is unavailable', () => { + const store = createCacheStore({ store: 'shared', sharedModule: '/tmp/not-found.cjs', logger: silentLogger }); + assert.equal(store.kind, 'memory'); + }); + it('reuses a persisted cache across service instances and skips failed reads', async () => { const file = tempCacheFile(); let positionCalls = 0; From 011a4a869168b7a90737e056b24d9355db26326f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 5 Sep 2026 09:29:35 +0000 Subject: [PATCH 3/4] Enforce strict ISO finance dates Co-authored-by: charles2ke <6725706+charles2ke@users.noreply.github.com> --- src/resolvers.js | 11 +++++++---- test/graphql.test.js | 8 ++++++++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/resolvers.js b/src/resolvers.js index f523fed..2c80259 100644 --- a/src/resolvers.js +++ b/src/resolvers.js @@ -1,14 +1,17 @@ import { GraphQLError } from 'graphql'; function isIsoDate(value) { - return Number.isFinite(new Date(value).getTime()); + const match = typeof value === 'string' && value.match(/^(\d{4}-\d{2}-\d{2})(?:T\d{2}:\d{2}:\d{2}(?:\.\d+)?(?:Z|[+-]\d{2}:\d{2}))?$/); + if (!match) return false; + const calendarDate = new Date(`${match[1]}T00:00:00.000Z`); + return Number.isFinite(calendarDate.getTime()) && calendarDate.toISOString().startsWith(match[1]) && Number.isFinite(new Date(value).getTime()); } function validateFinanceArgs(args, { requireTaxYear = false } = {}) { const issues = []; - if (args.from && !isIsoDate(args.from)) issues.push('from must be a valid ISO-8601 date'); - if (args.to && !isIsoDate(args.to)) issues.push('to must be a valid ISO-8601 date'); - if (args.from && args.to && isIsoDate(args.from) && isIsoDate(args.to) && new Date(args.from).getTime() > new Date(args.to).getTime()) { + if (args.from !== undefined && args.from !== null && !isIsoDate(args.from)) issues.push('from must be a valid ISO-8601 date'); + if (args.to !== undefined && args.to !== null && !isIsoDate(args.to)) issues.push('to must be a valid ISO-8601 date'); + if (isIsoDate(args.from) && isIsoDate(args.to) && new Date(args.from).getTime() > new Date(args.to).getTime()) { issues.push('from must be earlier than or equal to to'); } if (args.limit !== undefined && args.limit !== null && args.limit < 0) issues.push('limit must be greater than or equal to 0'); diff --git a/test/graphql.test.js b/test/graphql.test.js index 832d9c7..dbc454a 100644 --- a/test/graphql.test.js +++ b/test/graphql.test.js @@ -253,6 +253,14 @@ describe('GraphQL API', () => { assert.equal(invalidDate.errors[0].extensions.code, 'BAD_USER_INPUT'); assert.equal(invalidDate.errors[0].extensions.category, 'validation'); + const emptyDate = await execute('{ tradeHistory(from: "") { trades { id } } }'); + assert.equal(emptyDate.errors.length, 1); + assert.equal(emptyDate.errors[0].extensions.code, 'BAD_USER_INPUT'); + + const nonIsoDate = await execute('{ tradeHistory(from: "January 1, 2026") { trades { id } } }'); + assert.equal(nonIsoDate.errors.length, 1); + assert.equal(nonIsoDate.errors[0].extensions.code, 'BAD_USER_INPUT'); + const invalidRange = await execute('{ tradeHistory(from: "2026-02-01T00:00:00.000Z", to: "2026-01-01T00:00:00.000Z") { trades { id } } }'); assert.equal(invalidRange.errors.length, 1); assert.equal(invalidRange.errors[0].extensions.code, 'BAD_USER_INPUT'); From 4de1d96f0e4671180d2db8a879c0d917e0e40759 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 5 Sep 2026 09:32:22 +0000 Subject: [PATCH 4/4] Fix shared cache kind fallback Co-authored-by: charles2ke <6725706+charles2ke@users.noreply.github.com> --- README.md | 2 +- src/cache/index.js | 2 +- test/resilience.test.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 995c862..702f776 100644 --- a/README.md +++ b/README.md @@ -146,7 +146,7 @@ The running server loads connector settings from the environment via | `TAX_BREAK_ENDPOINT` | tax-break endpoint placeholder | `mock://tax-break` | | `TAX_BREAK_API_KEY` | tax-break credential placeholder | empty | | `FINANCE_CACHE_TTL_MS` | Resolver cache TTL | `1000` | -| `FINANCE_CACHE_STORE` | Cache strategy: `memory` or `file` (persistent) | `memory` | +| `FINANCE_CACHE_STORE` | Cache strategy: `memory`, `file` (persistent), or `shared` | `memory` | | `FINANCE_CACHE_FILE` | Cache file used when `FINANCE_CACHE_STORE=file` | `.cache/finance-cache.json` | | `FINANCE_CACHE_SHARED_MODULE` | Optional shared cache provider module path used when `FINANCE_CACHE_STORE=shared` | empty | | `FINANCE_HTTP_TIMEOUT_MS` | Per-request upstream timeout | `5000` | diff --git a/src/cache/index.js b/src/cache/index.js index c7ff05c..0cb2834 100644 --- a/src/cache/index.js +++ b/src/cache/index.js @@ -128,7 +128,7 @@ export function createCacheStore({ store = 'memory', file, sharedModule, logger if (typeof factory === 'function') { const sharedStore = factory({ logger }); if (sharedStore && typeof sharedStore.get === 'function' && typeof sharedStore.set === 'function' && typeof sharedStore.clear === 'function') { - return { kind: sharedStore.kind ?? 'shared', ...sharedStore }; + return { ...sharedStore, kind: sharedStore.kind ?? 'shared' }; } } logger.warn('shared cache module is invalid, falling back to memory', { sharedModule }); diff --git a/test/resilience.test.js b/test/resilience.test.js index 3d96bb8..da283ad 100644 --- a/test/resilience.test.js +++ b/test/resilience.test.js @@ -25,7 +25,7 @@ function tempSharedModule() { `module.exports.createSharedCacheStore = function createSharedCacheStore() { const entries = new Map(); return { - kind: 'shared', + kind: undefined, get(key) { const hit = entries.get(key); if (!hit || hit.expiresAt <= Date.now()) return undefined;