Skip to content

Commit ca6f77b

Browse files
authored
fix: api-key field names, dataset readiness check, punycode warning (#14)
* fix: api-key field names, dataset readiness check, punycode warning Three bugs/improvements that all surfaced from a real workflow run. 1. Fix api-key parsing GET /v1/apps/{id}/api-keys returns `{ api_key, api_key_2 }` per the OpenAPI schema, but the action was reading `{ primary, secondary }`, so smoke tests bailed with `Cannot run runtime probes: no API key returned for app …` immediately after a successful deploy. Update `getApiKeys` to return `{ api_key, api_key_2 }` and select the primary (or the secondary as a fallback). 2. Add dataset readiness check After the deployment reports `succeeded` and the runtime answers `isSpiceReady()`, poll `GET /v1/datasets?status=true` (per https://spiceai.org/docs/api/HTTP/get-datasets) until every dataset leaves the `initializing` state. Fail immediately if any dataset enters `error`, surfacing the `error_message` so the user knows which dataset is broken. New input `dataset-ready-timeout-seconds` (default 300, set to 0 to skip), new `datasets` action output, and a "Datasets" table in the GitHub job step summary. Status comparison is case-insensitive so the action handles both the lowercase values the public docs list and the PascalCase values the OSS runtime emits in its OpenAPI examples. 3. Stop the DEP0040 punycode warning `@spiceai/spice → node-fetch@2 → whatwg-url@5 → tr46@0.0.3` all `require("punycode")`, which Node resolves to its deprecated built-in. Install the userland `punycode@^2` package and add an esbuild `--alias:punycode=…` flag so the bundle ships the userland implementation instead. Tests: 90 passing (up from 83). New cases cover api-key field-name parsing, the runtime dataset poll loop (success / error / timeout / no-op), and the deploy orchestration paths for both successful and failing dataset checks. * fix: address PR review comments - Explicitly classify dataset statuses into ok/error/pending buckets so `shuttingdown` and any unrecognized values are treated as still-pending rather than slipping through as a false-positive "loaded". Only `ready`/`disabled`/`refreshing` count as terminal-ok; only `error` short-circuits with `DatasetReadinessError`. Timeout messages now include each non-terminal-ok dataset's status so the user can tell `initializing` from `shuttingdown` from a typo. - Make dataset readiness failures always fatal once the check is opted in (`dataset-ready-timeout-seconds > 0`), independent of `fail-on-test-error` (which only governs runtime-probe results). Match action.yml/README/CHANGELOG to that behavior. The opt-out is `dataset-ready-timeout-seconds: 0`. - Add deploy + runtime tests covering each new path: terminal-ok values beyond `ready`, `shuttingdown`/unknown handling, and the fail-on-test-error-false-but-dataset-error-still-fatal case.
1 parent 399858d commit ca6f77b

14 files changed

Lines changed: 525 additions & 75 deletions

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,16 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
88

99
### Added
1010
- Auto-capture a `repository` tag from `GITHUB_REPOSITORY` when set, sanitized to fit the API's tag-value rule (`/``_`). Users can override by setting `repository:` explicitly in the `tags` input.
11+
- New post-deploy dataset readiness check: poll `GET /v1/datasets?status=true` until every dataset reaches a terminal-ok state (`ready`, `disabled`, or `refreshing`); fail the job immediately on `error` or on timeout-while-pending — regardless of `fail-on-test-error`, which still only governs runtime-probe results. Statuses like `shuttingdown` and any unrecognized values are treated as still-pending so the loop never returns a false-positive "loaded". Configured via `dataset-ready-timeout-seconds` (default `300`, set `0` to skip). Dataset states are surfaced as a `datasets` action output and as a table in the GitHub job step summary.
1112

1213
### Changed
1314
- `parseTags` now validates tag values against the Spice Cloud API rule (alphanumeric plus `_@-`). Previously the action only enforced length, so values like `repo: foo/bar` would round-trip to the API and fail there with a generic 400.
1415
- Bump the action runtime from Node 20 to Node 24 (`runs.using: node24`). Node 20 actions are deprecated by GitHub and will be force-defaulted to Node 24 on June 2, 2026, with Node 20 removed from the runner on September 16, 2026. The build target, CI matrix, `engines.node`, and `.nvmrc` are aligned to Node 24.
1516

17+
### Fixed
18+
- The Spice Cloud `GET /v1/apps/{appId}/api-keys` response is `{ api_key, api_key_2 }`, but the action was reading `{ primary, secondary }` and bailing with `Cannot run runtime probes: no API key returned for app …` whenever runtime probes were enabled. Smoke tests now correctly retrieve the primary (or secondary) key.
19+
- Suppress the `(node:NNNN) [DEP0040] DeprecationWarning: The 'punycode' module is deprecated` runtime warning by aliasing the bare `punycode` specifier to the userland `punycode@2` package at bundle time. The transitive chain `@spiceai/spice → node-fetch@2 → whatwg-url@5 → tr46@0.0.3` previously resolved to Node's deprecated built-in.
20+
1621
## [1.0.0] — 2026-05-02
1722

1823
### Added

README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ Grant exactly the scopes for the features you use. The "All-in" row at the botto
105105
| `test-mcp-arguments` | no | `{}` | JSON-encoded arguments for the MCP tool call. |
106106
| `test-warmup-seconds` | no | `60` | Max wait for `isSpiceReady()` before running probes. |
107107
| `test-timeout-seconds` | no | `30` | Per-probe HTTP timeout. |
108+
| `dataset-ready-timeout-seconds` | no | `300` | Max wait for every dataset (via `GET /v1/datasets?status=true`) to reach a terminal-ok state (`ready`/`disabled`/`refreshing`). The job fails the moment any dataset enters `error`, or when the timeout elapses while any dataset is still pending — independent of `fail-on-test-error`. Set to `0` to skip the check. |
108109
| `runtime-url` | no | derived | Override probe base URL. By default derived from the app's region as `https://<region>-prod-aws-data.spiceai.io`. |
109110
| `fail-on-test-error` | no | `true` | Fail the job when any probe fails. |
110111
| `api-url` | no | `https://api.spice.ai` | Management API base URL. |
@@ -122,6 +123,7 @@ Grant exactly the scopes for the features you use. The "All-in" row at the botto
122123
| `deployment-status` | Final status (`queued`, `in_progress`, `succeeded`, `failed`). |
123124
| `deployment-created-at` | ISO 8601 timestamp the deployment was created. |
124125
| `test-results` | JSON array of `{ name, ok, durationMs, detail?, error? }`. |
126+
| `datasets` | JSON array of `{ name, status, from?, error?, error_message? }` from `GET /v1/datasets?status=true` after the deployment succeeds. |
125127

126128
## Examples
127129

__tests__/deploy.test.ts

Lines changed: 90 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ const baseInputs: ActionInputs = {
5151
oauthTokenUrl: "https://spice.ai/api/oauth/token",
5252
testWarmupSeconds: 0,
5353
testTimeoutSeconds: 30,
54+
datasetReadyTimeoutSeconds: 0,
5455
failOnTestError: true,
5556
};
5657

@@ -240,7 +241,7 @@ describe("runDeploy", () => {
240241
it("runs probes and fails when one fails (fail-on-test-error=true)", async () => {
241242
const listApps = vi.fn().mockResolvedValue([sampleApp]);
242243
const createDeployment = vi.fn().mockResolvedValue(succeededDeployment);
243-
const getApiKeys = vi.fn().mockResolvedValue({ primary: "rk_test" });
244+
const getApiKeys = vi.fn().mockResolvedValue({ api_key: "rk_test", api_key_2: null });
244245
const api = fakeApi({ listApps, createDeployment, getApiKeys });
245246

246247
const probeSql = vi.fn().mockResolvedValue<ProbeResult>({
@@ -268,7 +269,7 @@ describe("runDeploy", () => {
268269
it("skips probes when API key is unavailable but warns when fail-on-test-error=false", async () => {
269270
const listApps = vi.fn().mockResolvedValue([sampleApp]);
270271
const createDeployment = vi.fn().mockResolvedValue(succeededDeployment);
271-
const getApiKeys = vi.fn().mockResolvedValue({});
272+
const getApiKeys = vi.fn().mockResolvedValue({ api_key: null, api_key_2: null });
272273
const api = fakeApi({ listApps, createDeployment, getApiKeys });
273274

274275
const result = await runDeploy(api, {
@@ -309,7 +310,7 @@ describe("runDeploy", () => {
309310
it("runs all configured probes in order", async () => {
310311
const listApps = vi.fn().mockResolvedValue([sampleApp]);
311312
const createDeployment = vi.fn().mockResolvedValue(succeededDeployment);
312-
const getApiKeys = vi.fn().mockResolvedValue({ primary: "rk" });
313+
const getApiKeys = vi.fn().mockResolvedValue({ api_key: "rk", api_key_2: null });
313314
const api = fakeApi({ listApps, createDeployment, getApiKeys });
314315

315316
const calls: string[] = [];
@@ -352,4 +353,90 @@ describe("runDeploy", () => {
352353

353354
expect(calls).toEqual(["sql", "nsql", "chat", "search", "mcp"]);
354355
});
356+
357+
it("waits for datasets before probes and fails when any dataset is in error state", async () => {
358+
const listApps = vi.fn().mockResolvedValue([sampleApp]);
359+
const createDeployment = vi.fn().mockResolvedValue(succeededDeployment);
360+
const getApiKeys = vi.fn().mockResolvedValue({ api_key: "rk", api_key_2: null });
361+
const api = fakeApi({ listApps, createDeployment, getApiKeys });
362+
363+
const waitForDatasetsReady = vi.fn().mockRejectedValue(
364+
Object.assign(new Error("1 dataset(s) failed to load: foo: bad creds"), {
365+
name: "DatasetReadinessError",
366+
datasets: [{ name: "foo", status: "Error", error_message: "bad creds" }],
367+
}),
368+
);
369+
const probeSql = vi.fn();
370+
const fakeRuntime = {
371+
waitForReady: vi.fn().mockResolvedValue(undefined),
372+
waitForDatasetsReady,
373+
probeSql,
374+
} as unknown as RuntimeClient;
375+
376+
await expect(
377+
runDeploy(
378+
api,
379+
{ ...baseInputs, datasetReadyTimeoutSeconds: 60, testSql: "SELECT 1" },
380+
{ runtimeFactory: () => fakeRuntime },
381+
),
382+
).rejects.toThrow(/dataset.*failed to load/);
383+
384+
expect(waitForDatasetsReady).toHaveBeenCalledWith(60);
385+
expect(probeSql).not.toHaveBeenCalled();
386+
});
387+
388+
it("dataset readiness failures are fatal even when fail-on-test-error is false", async () => {
389+
// The opt-out for the dataset check is `dataset-ready-timeout-seconds: 0`.
390+
// `fail-on-test-error` only governs runtime-probe results.
391+
const listApps = vi.fn().mockResolvedValue([sampleApp]);
392+
const createDeployment = vi.fn().mockResolvedValue(succeededDeployment);
393+
const getApiKeys = vi.fn().mockResolvedValue({ api_key: "rk", api_key_2: null });
394+
const api = fakeApi({ listApps, createDeployment, getApiKeys });
395+
396+
const fakeRuntime = {
397+
waitForReady: vi.fn().mockResolvedValue(undefined),
398+
waitForDatasetsReady: vi.fn().mockRejectedValue(
399+
Object.assign(new Error("1 dataset(s) failed to load: foo: bad creds"), {
400+
name: "DatasetReadinessError",
401+
datasets: [{ name: "foo", status: "Error", error_message: "bad creds" }],
402+
}),
403+
),
404+
} as unknown as RuntimeClient;
405+
406+
await expect(
407+
runDeploy(
408+
api,
409+
{
410+
...baseInputs,
411+
datasetReadyTimeoutSeconds: 60,
412+
failOnTestError: false,
413+
},
414+
{ runtimeFactory: () => fakeRuntime },
415+
),
416+
).rejects.toThrow(/dataset.*failed to load/);
417+
});
418+
419+
it("returns dataset states when all datasets are ready", async () => {
420+
const listApps = vi.fn().mockResolvedValue([sampleApp]);
421+
const createDeployment = vi.fn().mockResolvedValue(succeededDeployment);
422+
const getApiKeys = vi.fn().mockResolvedValue({ api_key: "rk", api_key_2: null });
423+
const api = fakeApi({ listApps, createDeployment, getApiKeys });
424+
425+
const datasets = [
426+
{ name: "a", status: "Ready" },
427+
{ name: "b", status: "Ready" },
428+
];
429+
const fakeRuntime = {
430+
waitForReady: vi.fn().mockResolvedValue(undefined),
431+
waitForDatasetsReady: vi.fn().mockResolvedValue(datasets),
432+
} as unknown as RuntimeClient;
433+
434+
const result = await runDeploy(
435+
api,
436+
{ ...baseInputs, datasetReadyTimeoutSeconds: 60 },
437+
{ runtimeFactory: () => fakeRuntime },
438+
);
439+
440+
expect(result.datasets).toEqual(datasets);
441+
});
355442
});

__tests__/runtime.test.ts

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { beforeEach, describe, expect, it, vi } from "vitest";
22

33
vi.mock("@actions/core", () => ({
4+
debug: vi.fn(),
45
info: vi.fn(),
56
setSecret: vi.fn(),
67
warning: vi.fn(),
@@ -160,6 +161,171 @@ describe("RuntimeClient", () => {
160161
expect((init.headers as Record<string, string>).Authorization).toBe("Bearer k");
161162
});
162163

164+
it("getDatasets requests /v1/datasets?status=true with x-api-key", async () => {
165+
const fetchImpl = vi.fn().mockResolvedValue(
166+
new Response(JSON.stringify([{ name: "a", status: "Ready" }]), {
167+
status: 200,
168+
headers: { "content-type": "application/json" },
169+
}),
170+
);
171+
const rt = new RuntimeClient({
172+
apiKey: "k",
173+
baseUrl: "https://us-west-2-prod-aws-data.spiceai.io",
174+
warmupSeconds: 0,
175+
timeoutSeconds: 5,
176+
sdkFactory: () => makeSdk(),
177+
fetchImpl,
178+
});
179+
const result = await rt.getDatasets();
180+
expect(result).toEqual([{ name: "a", status: "Ready" }]);
181+
const [url, init] = fetchImpl.mock.calls[0]!;
182+
expect(url).toBe("https://us-west-2-prod-aws-data.spiceai.io/v1/datasets?status=true");
183+
expect(init.method).toBe("GET");
184+
expect((init.headers as Record<string, string>)["x-api-key"]).toBe("k");
185+
});
186+
187+
it("waitForDatasetsReady returns once all datasets are ready", async () => {
188+
const fetchImpl = vi
189+
.fn()
190+
.mockResolvedValueOnce(
191+
new Response(
192+
JSON.stringify([
193+
{ name: "a", status: "Initializing" },
194+
{ name: "b", status: "Ready" },
195+
]),
196+
{ status: 200, headers: { "content-type": "application/json" } },
197+
),
198+
)
199+
.mockResolvedValue(
200+
new Response(
201+
JSON.stringify([
202+
{ name: "a", status: "Ready" },
203+
{ name: "b", status: "Ready" },
204+
]),
205+
{ status: 200, headers: { "content-type": "application/json" } },
206+
),
207+
);
208+
const clock = makeClock();
209+
const rt = new RuntimeClient({
210+
apiKey: "k",
211+
baseUrl: "https://x.example",
212+
warmupSeconds: 0,
213+
timeoutSeconds: 5,
214+
sdkFactory: () => makeSdk(),
215+
fetchImpl,
216+
clock,
217+
});
218+
const datasets = await rt.waitForDatasetsReady(60);
219+
expect(datasets.every((d) => d.status === "Ready")).toBe(true);
220+
expect(fetchImpl).toHaveBeenCalledTimes(2);
221+
});
222+
223+
it("waitForDatasetsReady throws DatasetReadinessError on error state", async () => {
224+
const fetchImpl = vi
225+
.fn()
226+
.mockResolvedValue(
227+
new Response(
228+
JSON.stringify([{ name: "a", status: "Error", error_message: "auth failed" }]),
229+
{ status: 200, headers: { "content-type": "application/json" } },
230+
),
231+
);
232+
const rt = new RuntimeClient({
233+
apiKey: "k",
234+
baseUrl: "https://x.example",
235+
warmupSeconds: 0,
236+
timeoutSeconds: 5,
237+
sdkFactory: () => makeSdk(),
238+
fetchImpl,
239+
});
240+
await expect(rt.waitForDatasetsReady(60)).rejects.toMatchObject({
241+
name: "DatasetReadinessError",
242+
message: expect.stringContaining("auth failed"),
243+
});
244+
});
245+
246+
it("waitForDatasetsReady throws on timeout while still initializing", async () => {
247+
const fetchImpl = vi.fn().mockResolvedValue(
248+
new Response(JSON.stringify([{ name: "a", status: "Initializing" }]), {
249+
status: 200,
250+
headers: { "content-type": "application/json" },
251+
}),
252+
);
253+
const clock = makeClock();
254+
const rt = new RuntimeClient({
255+
apiKey: "k",
256+
baseUrl: "https://x.example",
257+
warmupSeconds: 0,
258+
timeoutSeconds: 5,
259+
sdkFactory: () => makeSdk(),
260+
fetchImpl,
261+
clock,
262+
});
263+
await expect(rt.waitForDatasetsReady(5)).rejects.toThrow(/did not finish loading/);
264+
});
265+
266+
it("waitForDatasetsReady accepts disabled and refreshing as terminal-ok", async () => {
267+
const fetchImpl = vi.fn().mockResolvedValue(
268+
new Response(
269+
JSON.stringify([
270+
{ name: "a", status: "Ready" },
271+
{ name: "b", status: "Disabled" },
272+
{ name: "c", status: "Refreshing" },
273+
]),
274+
{ status: 200, headers: { "content-type": "application/json" } },
275+
),
276+
);
277+
const rt = new RuntimeClient({
278+
apiKey: "k",
279+
baseUrl: "https://x.example",
280+
warmupSeconds: 0,
281+
timeoutSeconds: 5,
282+
sdkFactory: () => makeSdk(),
283+
fetchImpl,
284+
});
285+
const datasets = await rt.waitForDatasetsReady(60);
286+
expect(datasets.map((d) => d.name)).toEqual(["a", "b", "c"]);
287+
});
288+
289+
it("waitForDatasetsReady treats shuttingdown and unknown statuses as still-pending (does not return early)", async () => {
290+
const fetchImpl = vi.fn().mockResolvedValue(
291+
new Response(
292+
JSON.stringify([
293+
{ name: "a", status: "Ready" },
294+
{ name: "b", status: "ShuttingDown" },
295+
{ name: "c", status: "QuantumFlux" },
296+
]),
297+
{ status: 200, headers: { "content-type": "application/json" } },
298+
),
299+
);
300+
const clock = makeClock();
301+
const rt = new RuntimeClient({
302+
apiKey: "k",
303+
baseUrl: "https://x.example",
304+
warmupSeconds: 0,
305+
timeoutSeconds: 5,
306+
sdkFactory: () => makeSdk(),
307+
fetchImpl,
308+
clock,
309+
});
310+
await expect(rt.waitForDatasetsReady(5)).rejects.toThrow(
311+
/did not finish loading.*ShuttingDown.*QuantumFlux/s,
312+
);
313+
});
314+
315+
it("waitForDatasetsReady is a no-op when timeout is 0", async () => {
316+
const fetchImpl = vi.fn();
317+
const rt = new RuntimeClient({
318+
apiKey: "k",
319+
baseUrl: "https://x.example",
320+
warmupSeconds: 0,
321+
timeoutSeconds: 5,
322+
sdkFactory: () => makeSdk(),
323+
fetchImpl,
324+
});
325+
expect(await rt.waitForDatasetsReady(0)).toEqual([]);
326+
expect(fetchImpl).not.toHaveBeenCalled();
327+
});
328+
163329
it("probeSearch reports failures with body context", async () => {
164330
const fetchImpl = vi
165331
.fn()

action.yml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,16 @@ inputs:
149149
description: Per-probe HTTP timeout in seconds.
150150
required: false
151151
default: "30"
152+
dataset-ready-timeout-seconds:
153+
description: |
154+
Maximum seconds to wait for every dataset reported by `GET /v1/datasets?status=true`
155+
to reach a terminal-ok state (`ready`, `disabled`, or `refreshing`) before running
156+
runtime probes. The action fails the job immediately if any dataset enters `error`,
157+
or if the timeout elapses while datasets are still pending — regardless of
158+
`fail-on-test-error`, which only governs runtime-probe results. To disable the
159+
dataset check entirely, set this to `0`.
160+
required: false
161+
default: "300"
152162
runtime-url:
153163
description: |
154164
Override the runtime base URL probes connect to. When unset, the URL is
@@ -187,6 +197,8 @@ outputs:
187197
description: ISO 8601 timestamp the deployment was created.
188198
test-results:
189199
description: JSON array of `{ name, ok, durationMs, detail?, error? }` for each runtime probe.
200+
datasets:
201+
description: JSON array of `{ name, status, from?, error?, error_message? }` from `GET /v1/datasets?status=true` after deployment.
190202

191203
runs:
192204
using: node24

0 commit comments

Comments
 (0)