fix(api-gateway): poll a pre-agg job on the data source that built it

[ovr]

Check List

• Tests have been run in packages where changes have been made if available
• Linter has been run for changed code
• Tests for the changes have been added if not covered yet
• Docs have been added / updated if required

Issue Reference this PR resolves

Fixes #11615

Description of Changes Made (if issue reference is not provided)

#11617 stopped the Cannot read properties of undefined (reading 'getQueueDriver') crash by creating the queue client lazily, but the data source handed to isPartitionExist was still wrong, so the poll read a foreign queue and build failures never surfaced as failure: …. The gateway took it from metaConfigExtended().cubeDefinitions, which is the raw definition map — extends inheritance only exists on the built cube, so an inherited data_source reads as undefined and falls back to default while the build queued on the named one; it now uses job.dataSource, falling back to a resolved dataSource newly exposed on PreAggregationInfo, which also lets the expensive metaConfigExtended call drop off this polling endpoint. A posted job additionally returns one entry per built pre-aggregation (the partition plus every dependency) and each becomes its own token, but RefreshScheduler labelled only job[0] and QueryOrchestrator stamped every entry with preAggregations[0].preAggregationId — so entries 1..n were cached with dataSource: undefined and the gateway resolved the wrong pre-aggregation and external flag. Each result entry now carries the preAggregationId, dataSource and timezone of the descriptor it was built from, so a jobed build self-describes; this only bites builds with dependencies (lambda rollups, or a rollup on top of an originalSql pre-aggregation). Covered by new unit tests in cubejs-query-orchestrator (QueryOrchestrator.jobs.test.ts, PreAggregations.test.ts) and cubejs-api-gateway.

🤖 Generated with Claude Code

[claude]

Claude finished @ovr's task in 3m 20s —— View job

---

Review complete — approach is sound; 1 medium, 3 low. 2 inline comments posted.

Full review

• Read the PR diff (git diff origin/master...HEAD)
• Trace the data-source resolution path through gateway → OrchestratorApi → PreAggregations.isPartitionExist
• Verify CubeEvaluator.evaluatedCubes actually resolves extends inheritance (it does — prepareCube runs on compiled cubes, unlike metaConfigExtended().cubeDefinitions)
• Check RefreshScheduler job labelling and getPreAggsJobsList
• Review tests
• Check for existing review threads (none)
• Post review

Summary

The diagnosis holds up. preAggregations: dependencies.concat([partition]) puts the requested partition last, so the old job[0].dataSource = partition.dataSource stamped the first dependency with the partition's data source and left the partition itself unlabelled, while QueryOrchestrator stamped every entry with preAggregations[0].preAggregationId. Both are now per-entry, and the entries self-describe via preAggregationId/dataSource/timezone carried through LoadPreAggregationResult. The 'default' fallback in PreAggregations.ts:582 matches the one in CubeEvaluator.preAggregations() and in isPartitionExist(dataSource = 'default'), so the three agree.

Two things I liked: dropping metaConfigExtended (and its ad-hoc metaCache Map) off the polling endpoint is a real win — that call compiles the full meta on every poll of every token; and typing type on LoadPreAggregationResult where it was previously spread in untyped.

Findings

#	Severity	Where	Issue
1	Medium	gateway.ts:1223	Queue-status probe doesn't get the same fallback (inline)
2	Low	QueryOrchestrator.ts:255	Retained preAggregations[0] fallback still mislabels multi-entry jobs (inline)
3	Low	types/request.ts:190-191	PreAggJob.dataSource/timezone typed as required
4	Low	RefreshScheduler.ts:793	Fallback is now unreachable

3 — type honesty. PreAggJob declares timezone: string; dataSource: string, but the whole point of the new fallback is that cached jobs within the 24h TTL may have neither, and the test has to write dataSource: undefined as any to express that. Marking both optional would make the compiler enforce the fallback at every read site (including selector.dataSources at line 1153 and getPreAggregationQueueStates at 1223) instead of leaving it to review.

4 — dead branch. j.dataSource = j.dataSource || partition.dataSource can no longer take the right-hand side, since PreAggregations now always sets dataSource (falling back to 'default') and timezone comes from the descriptor. Harmless belt-and-braces, but if it's meant as a guard against a non-RefreshScheduler caller it'd read better with a comment saying so; otherwise a plain assignment from the entry is clearer about where the value now comes from.

Testing

Coverage is well-targeted at the three seams that broke — per-entry labelling in PreAggregations, the id fallback in QueryOrchestrator, and the data-source choice in the gateway — and the 'default'-fallback cases are covered on both sides. Gaps worth considering:

• No test exercises the actual regression end-to-end: a build whose descriptor list is [dependency, partition] where the two live on different data sources. The RefreshScheduler assertion added at RefreshScheduler.test.ts:908 asserts dataSource === 'default' for every job, which passes equally well under the old job[0]-only code for a single-data-source fixture. A fixture with a named data_source on one cube would actually fail on master.
• The extends-inheritance case from the issue (an inherited data_source reading as undefined off cubeDefinitions) isn't covered in cubejs-schema-compiler — a CubeEvaluator.preAggregations() test asserting dataSource on a pre-agg in a cube that inherits data_source via extends would lock in the fix at its root.
• Both new suites build instances via Object.create(Ctor.prototype) and poke private fields. Pragmatic for private methods, but it means a refactor that moves state into the constructor breaks the tests silently rather than the code.

Security / performance

Nothing of concern. No new input reaches a driver unsanitized — dataSource is a model-derived or previously-recorded identifier used to select a driver/queue, not interpolated into SQL. Performance is a net improvement per finding above.

Not verified

I could not run the test suites — the checkout has no installed dependencies (packages/cubejs-query-orchestrator/node_modules absent) and a full yarn install on this monorepo isn't feasible within this job. Findings are from reading the code, not from a failing run. CI results should be the source of truth for the tests themselves.

• branch [`core-813-pre-agg-jobs-crash`](https://github.com/cube-js/cube/tree/core-813-pre-agg-jobs-crash)

[claude]

Claude finished @ovr's task in 1m 34s —— View job

---

Re-review of 7799087 — prior medium finding is fixed; 2 low nits remain, no new inline comments

Full re-review

• Read the new commit 7799087
• Verify prior finding ReferenceError: self is not defined - Issue with whatwg-fetch when using the core lib with Node.js #1 (queue-status probe fallback) is addressed
• Re-check remaining findings (ResultSet pivot methods documentation  #3 type honesty, Self reference guard in schema compiler #4 dead branch)
• Resolve the now-stale thread on gateway.ts
• Post updated review

What changed

preAggregationsJobsGET now resolves the data source once, before the job.status branch (gateway.ts:1148-1150), and threads it into all three consumers — the response selector (:1157), getPreAggJobQueueStatus (:1174) and getPreAggJobResultStatus (:1191). That's exactly the fix my medium finding asked for, and it closes the case where a legacy cached job probed the default queue, found nothing, and reported a still-scheduled build as missing_partition. The TODO(1.8) marking the fallback as a TTL-bounded compatibility shim is the right way to keep it from becoming permanent.

The new test at index.test.ts:1303 is the strongest one in the PR: it drives the real preAggregationsJobsGET end to end with dataSource: undefined and status: 'scheduled', and asserts all three seams agree on model_ds — the selector, the queue call, and isPartitionExist. That's a test that fails on master and on the previous commit of this branch, which the earlier single-seam test did not.

I resolved my gateway.ts thread — it's addressed and outdated.

Remaining, both low and both carried over

1 — PreAggJob.dataSource/timezone are typed as required (types/request.ts:190-191). The whole point of the fallback is that a cached job may have neither, and the new test still has to write dataSource: undefined as any to express the case the production code handles. Marking both optional would make the compiler point at every read site instead of leaving it to review — and would make the TODO(1.8) removal a compile error rather than a silent behaviour change. Same applies to JobedPreAggregation in RefreshScheduler.ts:64-65, where j.dataSource || partition.dataSource reads as dead code against a required string.

Fix this →

2 — the model is scanned twice on the fallback path. gateway.ts:1149 does (await compiler.preAggregations()).find(pa => pa.id === job.preagg) and getPreAggJobResultStatus:1400 repeats the identical call and find. cubeEvaluator.preAggregations() walks every cube and calls evaluatePreAggregationReferences per pre-agg, so this doubles that walk for exactly the legacy jobs. Resolving the PreAggregationInfo once in the handler and passing it down would remove both the second scan and the duplicated lookup — and would collapse the dataSource param and the pre_agg_not_found check into one thing. Only affects jobs missing dataSource, so it's genuinely low.

Not worth a comment, but noting for symmetry: getPreAggJobQueueStatus(…, dataSource?: string) is optional while getPreAggJobResultStatus(…, dataSource: string | undefined, …) is required-but-nullable, for the same value from the same caller. Picking one shape reads better.

The QueryOrchestrator.ts:255 thread has been resolved by the author; I'm not re-raising it.

Testing

The RefreshScheduler.test.ts:907 comment reword is accurate about what the assertion proves (every entry gets a token), which is better than the previous wording that implied it proved per-entry data-source labelling. The gap I noted last round still stands: no fixture has a cube on a named data_source, so expect(job?.dataSource).toEqual('default') passes under the old job[0]-only code too. And the extends-inheritance root cause still has no CubeEvaluator.preAggregations() test in cubejs-schema-compiler.

Not verified

Same as last round — I could not run the suites (no installed node_modules in this checkout, full yarn install on the monorepo isn't feasible in this job). Findings are from reading the code; CI is the source of truth for the tests.

• branch [`core-813-pre-agg-jobs-crash`](https://github.com/cube-js/cube/tree/core-813-pre-agg-jobs-crash)

[codecov]

Codecov Report

❌ Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.38%. Comparing base (ab48039) to head (7799087).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
...es/cubejs-server-core/src/core/RefreshScheduler.ts	33.33%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #11666      +/-   ##
==========================================
+ Coverage   58.20%   58.38%   +0.18%     
==========================================
  Files         237      237              
  Lines       18852    18853       +1     
  Branches     3825     3830       +5     
==========================================
+ Hits        10973    11008      +35     
+ Misses       7323     7283      -40     
- Partials      556      562       +6     

Flag	Coverage Δ
cube-backend	58.38% <77.77%> (+0.18%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.