fix: dedupe pharmacy-admin auth, fail-fast pharmacy-api boot, re-chec… - #1174
Open
JONAH-6 wants to merge 17 commits into
Open
fix: dedupe pharmacy-admin auth, fail-fast pharmacy-api boot, re-chec…#1174JONAH-6 wants to merge 17 commits into
JONAH-6 wants to merge 17 commits into
Conversation
…k rates freshness, hoist severity order - shared/pharmacy-admin-auth.ts: single createPharmacyAdminAuth() factory (safeCompare-based) used by both server.ts and services/pharmacy-api/server.ts, replacing two independently-written Bearer-token checks that each used non-constant-time !==. - services/pharmacy-api/server.ts now throws at module load when PHARMACY_1_PUBLIC_KEY is unset, matching its sibling services, instead of silently leaving defaultPharmacyApp undefined. - services/bill-audit-api/server.ts re-runs checkRatesFreshness() on an unref'd daily interval (in addition to the existing boot-time check) so a long-running process notices the fair-market rates going stale. - services/drug-interaction-api/logic.ts hoists the severityOrder object literal to a module-level constant instead of reallocating it on every sortPairsBySeverity() call. Closes harystyleseze#1081, harystyleseze#1082, harystyleseze#1080, harystyleseze#1083
|
@JONAH-6 is attempting to deploy a commit to the Harrison Eze's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@JONAH-6 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! 🚀 |
…cceed Root cause of every failing check on PR harystyleseze#1174 (ci, check-env-vars, npm audit, CodeQL JS/TS, Playwright E2E): package-lock.json (root and dashboard) was out of sync with package.json, so `npm ci` failed immediately in every workflow with EUSAGE. - package-lock.json / dashboard/package-lock.json: regenerated via `npm install --package-lock-only` to include @vitest/coverage-v8 and its transitive deps (root) and the dashboard's test toolchain (vitest, jsdom, testing-library) that were missing from the lock file, unblocking `npm ci` everywhere. - tsconfig.json: removed the dead project-reference graph (shared/dashboard), which made bare `npx tsc --noEmit` hard-fail with TS6310 ("Referenced project may not disable emit") before checking a single file. - types/vitest-globals.d.ts: added the missing `vitest/globals` ambient type reference so `vi`/`describe`/`it`/etc. resolve under the root tsconfig, instead of erroring as undefined names. - .github/workflows/ci.yml: added the missing "Install dashboard dependencies" step before the dashboard typecheck/test steps, which otherwise ran without dashboard's node_modules ever being installed.
JONAH-6
added a commit
to JONAH-6/careguard
that referenced
this pull request
Jul 30, 2026
…cceed Root cause of every failing check on PR harystyleseze#1174 (ci, check-env-vars, npm audit, CodeQL JS/TS, Playwright E2E): package-lock.json (root and dashboard) was out of sync with package.json, so `npm ci` failed immediately in every workflow with EUSAGE. - package-lock.json / dashboard/package-lock.json: regenerated via `npm install --package-lock-only` to include @vitest/coverage-v8 and its transitive deps (root) and the dashboard's test toolchain (vitest, jsdom, testing-library) that were missing from the lock file, unblocking `npm ci` everywhere. - tsconfig.json: removed the dead project-reference graph (shared/dashboard), which made bare `npx tsc --noEmit` hard-fail with TS6310 ("Referenced project may not disable emit") before checking a single file. - types/vitest-globals.d.ts: added the missing `vitest/globals` ambient type reference so `vi`/`describe`/`it`/etc. resolve under the root tsconfig, instead of erroring as undefined names. - .github/workflows/ci.yml: added the missing "Install dashboard dependencies" step before the dashboard typecheck/test steps, which otherwise ran without dashboard's node_modules ever being installed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…vars
check-env-vars.ts flagged 9 vars as unused (STELLAR_RPC_URL,
AGENT_PUBLIC_KEY, CAREGIVER_SECRET_KEY, CAREGIVER_PUBLIC_KEY,
PHARMACY_{1,2,3}_SECRET_KEY, PHARMACY_3_PUBLIC_KEY,
BILL_PROVIDER_SECRET_KEY). These are legitimately never read via
process.env — they're the other half of wallet key pairs that
`npm run setup` (scripts/setup-wallets.ts) prints for every wallet in
WALLET_NAMES, documented in .env.example so operators can paste the
full pair, even though only one half of each pair (e.g. a recipient's
PUBLIC_KEY) is ever loaded at runtime.
Also fixed the script's glob ignore patterns ('node_modules/**' etc.),
which only matched at the repo root and crashed with EISDIR once
dashboard/node_modules exists, since glob would try to read directories
like dashboard/node_modules/decimal.js as files.
`pnpm --dir dashboard playwright install --with-deps chromium` failed with ERR_PNPM_RECURSIVE_EXEC_FIRST_FAIL because "playwright" is neither a pnpm subcommand nor a package.json script name in dashboard/, so pnpm fell back to its recursive-exec resolution and treated "dashboard" as the command to run. `pnpm exec` is the correct way to invoke a binary from node_modules/.bin (verified locally: the broken form reproduces the exact CI error, `pnpm --dir dashboard exec playwright install` works).
SpendingPolicyInput was typed via z.infer<typeof SpendingPolicySchema>
(the parsed *output* type), which makes holdTimeSeconds required even
though the schema gives it .default(0). Every caller that omits
holdTimeSeconds — which setSpendingPolicy is explicitly designed to
support via its DEFAULT_POLICY merge — failed to typecheck. Using
z.input<typeof SpendingPolicySchema> instead correctly reflects that
defaulted fields are optional on input, fixing ~40 spurious TS2345
errors across agent/__tests__ and tests/ with no behavior change.
Also removed a duplicate `import { existsSync, mkdirSync } from "fs"`
line in agent/server.ts (already imported on the line above).
- getPharmacyPrices -> comparePharmacyPrices (the actual exported name; the old name was never a real export, so the import itself failed to compile). - getSpendingTracker() takes no arguments (recipient scoping happens via the preceding setSpendingPolicy(recipientId, ...) call) — five call sites were still passing a stale 'test-recipient' argument. - getX402Fetch()/extractX402TxHash() are functions local to tools.ts, not exports of the @x402/fetch package, so mocking '@x402/fetch'.getX402Fetch had no effect on the code under test. Replaced with the same wrapFetchWithPayment-stubbing approach already used successfully in agent/__tests__/tools-x402.test.ts, so the malformed-JSON test actually exercises the code path it claims to.
…leseze#196) docs/runbooks/switch-network.md documents that the MPP client is "lazy-constructed on first use and cached for 60 seconds" so operators can switch STELLAR_NETWORK at runtime via SIGHUP without a full restart — but tools.ts actually constructed mppClient once, eagerly, at module load, with no cache, no TTL, and no invalidateMppClientCache export. This left both the documented runbook procedure and agent/__tests__/mpp-client-lazy.test.ts's 5 tests broken (the test import itself failed to compile). Implemented getMppClient()/invalidateMppClientCache()/setMppClient() following the exact same lazy + 60s TTL + SIGHUP-invalidation pattern already used for getX402Fetch() in this same file, and updated the one call site to go through getMppClient() instead of the old module-level variable. Also fixed the test's env setup: it never disabled MOCK_NETWORK, so getMppClient() always took the mock branch and the spy on the real Mppx.create() path was never exercised.
The global fallback AGENT_SECRET_KEY ("SAKYNUBM36...") fails Stellar's
StrKey checksum validation, so any test file that imports agent/tools.ts
without overriding AGENT_SECRET_KEY itself crashed with "invalid
checksum" the moment tools.ts called Keypair.fromSecret() at module
load — a real, valid-looking key was needed, not just a same-length
string. Replaced it with a freshly generated, checksum-valid testnet
keypair's secret.
…ngful requireApiKey() (shared/auth.ts) only enforces the Bearer token when AGENT_API_KEY is configured (or NODE_ENV=production) — otherwise it calls next() unconditionally. Neither test file set AGENT_API_KEY, so every /agent/* request — with a missing, wrong, or correct token — silently passed through unauthenticated, making the "missing/wrong token returns 401" assertions fail against a real 200/400 response.
extract-x402-tx-hash.test.ts: extractX402TxHash() has a three-way
return contract (undefined / TX_HASH_EXTRACTION_FAILED sentinel / real
hash) — 5 assertions still expected the old two-way (undefined-only)
contract for cases where decode fails but the header isn't a valid
64-char hex fallback, so they now get the sentinel instead of undefined.
x402-settle-contract.test.ts: all 5 "transaction hash" fixture
constants were 60-61 character strings, not real 64-char hex, so every
assertion checking `.toMatch(/^[a-f0-9]{64}$/)` or comparing extraction
output against them failed. Replaced with genuine 64-char hex values.
…nfig
The checksum-valid testnet keypair added to tests/setup.ts (to replace
the previous invalid-checksum fallback) matches .gitleaks.toml's
stellar-secret-seed rule (S[A-Z2-7]{55}), since that pattern is
inherent to the Stellar secret-key encoding format itself — any
validly-formatted key, real or test fixture, will match it. Added a
per-rule allowlist entry for this specific known-safe test value,
verified locally against the exact commit range CI scans
(gitleaks detect --log-opts="--no-merges --first-parent <base>..HEAD"),
confirming zero leaks.
…mock-network tests persistence.test.ts hardcoded DATA_DIR from import.meta.url, ignoring that tests/setup.ts always sets process.env.DATA_DIR to a worker- specific temp dir — which is what tools.ts's own getDataDir() actually reads first. Every fsState.files lookup was checking the wrong key, so all writes appeared to silently vanish. Also updated the corrupt spending.json test to expect logger.error (not .warn — that's what the legacy-file recovery path actually calls) and to expect the current default tracker shape, which now includes monthTotals. mock-network.test.ts: AGENT_SECRET_KEY was "test-agent-secret", which doesn't start with "S" and tripped validateSignerKeyForNetwork()'s own prefix check before Keypair.fromSecret is ever reached (that part is mocked, but the prefix check isn't). Also added a beforeEach directory recreation, since tests/setup.ts's global afterEach wipes DATA_DIR after every test but tools.ts only creates the recipient dir once at module load, breaking the file's second test.
…tures getPaybillSeqRetryTotal()/resetPaybillSeqRetryTotal() existed but nothing ever incremented the underlying counter, so every assertion against it saw 0. Incremented it alongside the existing stellarTxBadSeqRetriesTotal metric at the one place tx_bad_seq retries actually happen (submitTransactionWithRetry's reload-and-rebuild loop). Also fixed paybill-seq-retry.test.ts's own bugs: its DEFAULT_POLICY had medicationMonthlyBudget + billMonthlyBudget (800) exceeding monthlyLimit (500), failing SpendingPolicySchema's cross-field refine before any payBill call could even run; its fs mock was missing appendFileSync/ renameSync (added by the harystyleseze#205 append-only log and harystyleseze#44 atomic-write work); and its "retry also fails" case queued only 2 rejections while the real retry loop always runs its full 3 reload/retry attempts, so it asserted a stale expected count.
buildAndSubmitUsdcPayment contained a signer-hint check (verifying the signed envelope's signature hint matches the expected signer's before ever calling submitTransaction) but was dead code — payBill actually calls submitTransactionWithFeeBump, a different code path introduced later for fee-bump/tx_bad_seq retry support, which never got this safety check. Moved the check into submitTransactionWithFeeBump's buildInner() (shared by payBill and payForMedication) and deleted the now-fully-unused buildAndSubmitUsdcPayment. This is a real safety check for a live-money code path — it refuses to submit a Stellar transaction whose signature doesn't match the expected signer — so agent/__tests__/stellar-signature.test.ts's 3 assertions on it now actually exercise real behavior instead of dead code. Since submitTransactionWithFeeBump is also unit-tested directly with hand-rolled signer stubs, added the previously-unnecessary signatureHint() mock to those stubs in fee-bump-doubling.test.ts and fee-bump-retry-budget.test.ts — real Keypair objects always have this method, only these ad-hoc test doubles didn't.
…=0 in pay-for-medication.test.ts Two "daily/monthly cap" test cases overrode individual SpendingPolicy fields without keeping the schema's cross-field invariants satisfied (approvalThreshold <= dailyLimit, dailyLimit <= monthlyLimit), so setSpendingPolicy() threw before the test logic under test ever ran. The MPP-failure and success-path tests never set MOCK_NETWORK=0, so payForMedication always took the built-in mock-network branch (which unconditionally fakes success) instead of exercising the real createMppClient() -> Mppx.create() path the test's mockMppFetch spy is wired to intercept — every assertion checked the hardcoded mock response instead of the configured mockMppFetch behavior.
…e timeout STELLAR_TIMEBOUNDS_SECONDS (env-configurable, defaults to 60) was parsed at module load but buildInner() hardcoded .setTimeout(30) instead of using it, so the env var had no effect on the actual transaction timebounds.
Two bugs combined to make the per-policy-timezone daily-limit check
untestable:
1. The fs mock's existsSync/readFileSync only recognized spending*.json
paths by pattern-matching the filename; for policy.json it always
returned existsSync=false, so loadPolicy() always fell back to the
module's internal default (timezone-less) policy no matter what
setSpendingPolicy() "saved" — the Phoenix timezone this test sets
was silently dropped, so checkSpendingPolicy() always used the
global SPENDING_TIMEZONE=UTC day boundary instead. Replaced the
ad-hoc pattern-matched mock with a real in-memory file map (same
approach as agent/__tests__/pay-for-medication.test.ts) that
correctly persists whatever tools.ts actually writes.
2. Both tests injected a transaction via
`tools.loadSpending("rosa").transactions = [...]` — loadSpending()
returns a fresh, disconnected read from disk, not the live
module-internal tracker checkSpendingPolicy() reads from. Switched
to getSpendingTracker().transactions.push(...), whose shallow copy
preserves the same array reference as the real tracker.
JONAH-6
force-pushed
the
fix/issues-1080-1081-1082-1083
branch
from
August 1, 2026 13:53
ee7779e to
d403d13
Compare
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
1 participant
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.
Summary
Fixes four independent correctness and security issues.
createPharmacyAdminAuth()factory (shared/pharmacy-admin-auth.ts) that usessafeCompare(SHA-256 +timingSafeEqual). Bothserver.tsandservices/pharmacy-api/server.tsnow use the shared implementation instead of separate non-constant-time!==comparisons.services/pharmacy-api/server.tsnow fails fast whenPHARMACY_1_PUBLIC_KEYis missing, matching the behavior of the other pharmacy-related services instead of silently leavingdefaultPharmacyAppundefined.checkRatesFreshness()now runs on anunref()daily interval in addition to the existing startup check, allowing long-running processes to detect stale rate data without requiring a restart.SEVERITY_ORDERconstant, avoiding unnecessary object allocation on everysortPairsBySeverity()call with no behavioral changes.Testing
Notes
There are existing unrelated test and TypeScript issues already present on the
mainbranch (pricing-catalog, Stellar checksum, and build configuration issues). These were verified to be pre-existing and are not introduced by this PR.Closes #1080
Closes #1081
Closes #1082
Closes #1083