Skip to content

Commit d350935

Browse files
committed
fix: split OSV activity env reads so derive doesn't require sync vars
Cursor pointed out that osvDeriveCriticalFlag was calling getActivityConfig() and eagerly validating OSV_BULK_BASE_URL, OSV_TMP_DIR, and OSV_BATCH_SIZE — env vars only the sync activity uses. Running the derive activity in isolation (e.g. for testing or debugging) failed with a misleading "Missing required env var" pointing at vars the derive never needed. Split into getSyncConfig() and getDeriveConfig(), each activity reads only its own env. Matches the same shape applied to proxyActivities in workflows.ts last round — sync and derive are independent activities and should have independent contracts. No behavior change in normal scheduled runs (the workflow sets up the full env anyway); the cleanup is for isolated/debug paths. 72 unit tests still pass. Signed-off-by: Joan Reyero <joan@reyero.io>
1 parent 2510e55 commit d350935

1 file changed

Lines changed: 14 additions & 7 deletions

File tree

services/apps/packages_worker/src/osv/activities.ts

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,25 @@ import { upsertAdvisoryBatch } from './upsertAdvisory'
1515

1616
const log = getServiceChildLogger('osv-sync')
1717

18-
// Read at activity invocation time, not at module load — keeps tests that
19-
// import this file from the sandboxed workflow bundle context working without
20-
// the env being set.
21-
function getActivityConfig() {
18+
// Per-activity env readers. Each activity reads only the env vars it actually
19+
// needs at invocation time, so e.g. running the derive activity in isolation
20+
// for testing doesn't require the sync-only OSV_BULK_BASE_URL / OSV_TMP_DIR /
21+
// OSV_BATCH_SIZE to be set. Read at invocation time (not module load) so the
22+
// sandboxed workflow bundle that imports this file for type discovery doesn't
23+
// require any env to be set.
24+
25+
function getSyncConfig() {
2226
return {
2327
bulkBaseUrl: required('OSV_BULK_BASE_URL'),
2428
tmpDir: required('OSV_TMP_DIR'),
2529
upsertBatchSize: requirePositiveInt('OSV_BATCH_SIZE'),
26-
deriveBatchSize: requirePositiveInt('OSV_DERIVE_BATCH_SIZE'),
2730
}
2831
}
2932

33+
function getDeriveConfig() {
34+
return { deriveBatchSize: requirePositiveInt('OSV_DERIVE_BATCH_SIZE') }
35+
}
36+
3037
function required(name: string): string {
3138
const value = process.env[name]
3239
if (!value) throw new Error(`Missing required environment variable: ${name}`)
@@ -78,7 +85,7 @@ export async function osvSyncEcosystem(
7885
input: OsvSyncEcosystemInput,
7986
): Promise<OsvSyncEcosystemResult> {
8087
const { ecosystem, allowedEcosystems } = input
81-
const config = getActivityConfig()
88+
const config = getSyncConfig()
8289
// OSV's bucket uses case-sensitive paths (Maven/all.zip, not maven/all.zip),
8390
// so `ecosystem` (used for the URL) keeps OSV's canonical case. The allowlist
8491
// is matched in parseOsvRecord after lowercasing each record's ecosystem per
@@ -159,7 +166,7 @@ export interface OsvDeriveCriticalFlagResult {
159166
// safe; re-running clears stale FALSE→TRUE and TRUE→FALSE transitions
160167
// identically.
161168
export async function osvDeriveCriticalFlag(): Promise<OsvDeriveCriticalFlagResult> {
162-
const config = getActivityConfig()
169+
const config = getDeriveConfig()
163170
const start = Date.now()
164171
const qx: QueryExecutor = await getPackagesDb()
165172
const { flipped, cleared } = await deriveCriticalFlag(qx, config.deriveBatchSize)

0 commit comments

Comments
 (0)