Pre-v1.0.0 polish: tags as map, fix org slug, clearer OAuth scopes#8
Closed
lukekim wants to merge 2 commits into
Closed
Pre-v1.0.0 polish: tags as map, fix org slug, clearer OAuth scopes#8lukekim wants to merge 2 commits into
lukekim wants to merge 2 commits into
Conversation
Two regressions came in via Dependabot bumps on trunk: - @biomejs/biome 1.x → 2.x renamed config keys (`files.ignore` → `files.includes` with negation patterns, `overrides[*].include` → `overrides[*].includes`, top-level `organizeImports` moved into `assist.actions.source.organizeImports`). Update biome.json to the v2 schema and reorder a stale import that the v2 organizer flagged. - @actions/core 1.x → 3.x is ESM-only and broke the CJS TypeScript build (TS1479). Pin it back to ^1.11.1 and ignore future major bumps in Dependabot until we migrate the project to ESM.
- Refactor the `tags` input to accept a YAML block mapping (the canonical workflow form) or a JSON object string instead of the prior `KEY=VALUE` lines. Tag keys still merge into the app's existing tags on every run. - Correct the `spiceai/spice-cloud-deploy-action` slug to `spicehq/spice-cloud-deploy-action` everywhere it appeared (README badges + examples, package.json metadata, examples/, etc.) so consumers can copy/paste a working `uses:` line at v1. - Replace the duplicated "Required scopes" tail-of-document section with a single clearer "Scope cheat sheet" right next to the OAuth client setup steps, so it's obvious which scopes to grant.
There was a problem hiding this comment.
Pull request overview
Release-readiness polish ahead of v1.0.0, primarily improving the ergonomics and documentation of app tags and OAuth scopes, and fixing repository/org references for copy/paste users.
Changes:
- Switch the
tagsinput from multilineKEY=VALUEto a YAML/JSON map, with updated parsing, tests, and examples. - Replace
spiceai/withspicehq/across docs/examples/package metadata to avoid brokenuses:references. - Clarify OAuth scope documentation and update toolchain/dependency config (Biome v2 schema,
@actions/corepin + Dependabot ignore).
Reviewed changes
Copilot reviewed 13 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/tags.ts |
Implements new YAML/JSON map parsing for tags with validation. |
src/deploy.ts |
Import ordering cleanup (aligns with formatter/organizer). |
package.json |
Updates repo URLs and pins @actions/core to v1. |
package-lock.json |
Reflects dependency graph changes from the @actions/core pin. |
biome.json |
Migrates config to Biome v2 schema and updates organize-imports settings. |
action.yml |
Updates tags input description/examples to YAML/JSON map form. |
README.md |
Fixes uses: org slug and consolidates OAuth scope guidance into a cheat sheet. |
CHANGELOG.md |
Notes the new accepted tags formats. |
examples/basic.yml |
Updates uses: and keeps examples aligned with new tags format. |
examples/full.yml |
Updates uses: and converts tags to YAML map form. |
.github/workflows/test-action.yml |
Updates test workflow to use new tags YAML map format. |
.github/dependabot.yml |
Ignores major bumps for @actions/core due to CJS/ESM constraints. |
__tests__/tags.test.ts |
Adds coverage for YAML map + JSON object parsing and error cases. |
__tests__/deploy.test.ts |
Updates deploy test to the new tags input format. |
Comments suppressed due to low confidence (1)
package.json:30
- After downgrading
@actions/coreto v1,@actions/http-clientnow depends onundici^5.x, but the repo-leveloverrides.undicistill forces^6.x(seepackage-lock.jsonresolvingundici@6.25.0). This major-version override could break@actions/http-clientat runtime. Consider removing theundicioverride or adjusting it to a version range compatible with the@actions/*v1 dependency tree.
"@actions/core": "^1.11.1",
"@spiceai/spice": "^3.1.0"
},
"overrides": {
"undici": "^6.23.0"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+65
to
+76
| const colon = line.indexOf(":"); | ||
| if (colon === -1) { | ||
| throw new InputValidationError( | ||
| `tags line ${i + 1}: key "${key}" must start with a letter and contain only letters, numbers, and "_:./-".`, | ||
| `tags line ${i + 1}: expected "key: value" (got "${line.trim()}").`, | ||
| ); | ||
| } | ||
| if (key in tags) { | ||
|
|
||
| const key = line.slice(0, colon).trim(); | ||
| const value = stripQuotes(line.slice(colon + 1).trim()); | ||
|
|
||
| validateKey(key, `tags line ${i + 1}`); | ||
| if (key in out) { |
Comment on lines
+76
to
+78
| it("rejects JSON arrays", () => { | ||
| // Arrays don't start with `{`, so the parser falls through to YAML parsing. | ||
| // This test ensures wrapping a literal `{` array-like in JSON fails clearly. |
Contributor
Author
lukekim
added a commit
that referenced
this pull request
May 2, 2026
Bundles the work from #7 and #8 into a single commit so trunk lands release-ready in one merge. CI / build - Migrate biome.json to the Biome 2.x schema (`files.includes` with negation patterns, `overrides[*].includes`, `assist.actions.source.organizeImports`). - Reorder a stale import in src/deploy.ts that the v2 organizer flagged. - Pin @actions/core to ^1.11.1 — 3.x is ESM-only and breaks the current CJS bundle. Add a Dependabot ignore for major bumps until the project is migrated to ESM. Action UX - `tags` input now accepts a YAML block mapping (the canonical workflow form) or a JSON object string, instead of the prior multi-line KEY=VALUE format. Tag keys still merge into the app's existing tags on every run. - Update action.yml description, README, and example workflows to the new tag form. Docs - Correct the GitHub slug from `spiceai/spice-cloud-deploy-action` to `spicehq/spice-cloud-deploy-action` everywhere it appeared (README badges + examples, package.json metadata, examples/), so a copy/pasted `uses:` line resolves to the published action at v1. - Replace the duplicated tail-of-document "Required scopes" table with a single "Scope cheat sheet" right under the OAuth client setup steps, including an "All-in (recommended for a single CI client)" row that spells out exactly which scopes to grant. Tests - New `parseTags` cases cover the YAML form, JSON form, quoted values, duplicates, and validation errors. - Total: 70 unit tests, all green.
lukekim
added a commit
that referenced
this pull request
May 2, 2026
* Prepare repo for v1.0.0 release Bundles the work from #7 and #8 into a single commit so trunk lands release-ready in one merge. CI / build - Migrate biome.json to the Biome 2.x schema (`files.includes` with negation patterns, `overrides[*].includes`, `assist.actions.source.organizeImports`). - Reorder a stale import in src/deploy.ts that the v2 organizer flagged. - Pin @actions/core to ^1.11.1 — 3.x is ESM-only and breaks the current CJS bundle. Add a Dependabot ignore for major bumps until the project is migrated to ESM. Action UX - `tags` input now accepts a YAML block mapping (the canonical workflow form) or a JSON object string, instead of the prior multi-line KEY=VALUE format. Tag keys still merge into the app's existing tags on every run. - Update action.yml description, README, and example workflows to the new tag form. Docs - Correct the GitHub slug from `spiceai/spice-cloud-deploy-action` to `spicehq/spice-cloud-deploy-action` everywhere it appeared (README badges + examples, package.json metadata, examples/), so a copy/pasted `uses:` line resolves to the published action at v1. - Replace the duplicated tail-of-document "Required scopes" table with a single "Scope cheat sheet" right under the OAuth client setup steps, including an "All-in (recommended for a single CI client)" row that spells out exactly which scopes to grant. Tests - New `parseTags` cases cover the YAML form, JSON form, quoted values, duplicates, and validation errors. - Total: 70 unit tests, all green. * fix: address PR review comments - parseBlockMap duplicate check now uses Object.hasOwn() so prototype- chain property names like `toString` and `constructor` aren't falsely rejected as duplicates. - Drop ':' from TAG_KEY_PATTERN. The block-map parser splits on the first ':', so a tag key containing ':' (e.g. `foo:bar`) couldn't be expressed in YAML form anyway. Aligning the JSON form keeps validation consistent across both input styles. Also rewords the validation error message to match the trimmed character set. - Rename the misleading "rejects JSON arrays" test to make clear it rejects non-string JSON values; add a separate case for a root-level JSON array (which falls through to the YAML parser); add a regression test for the prototype-chain dupe-check fix.
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
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
Three release-readiness fixes for the
v1.0.0tag. Stacks on top of #7 (Biome v2 +@actions/corepin); merge that first for the cleanest diff.1.
tagsinput is now a YAML/JSON map (was multi-lineKEY=VALUE)Tag pairs are by nature a map of strings, so they should look like one in the workflow. Now:
JSON form (single-line) is also accepted:
tags: '{\"environment\":\"production\"}'.Implementation:
src/tags.tsparses either form intoRecord<string,string>. Lines starting with#are comments. Same key/value validation rules as before (key must start with a letter; value ≤ 256 chars). Tags continue to merge with the app's existing tags on every run.Updated examples, action.yml description, README, and the
Test actionworkflow.2.
spiceai/→spicehq/everywhereThe repo lives at
spicehq/spice-cloud-deploy-action, but README badges,uses:examples, and package.json links all pointed atspiceai/. Anyone copy/pasting auses:from the README at v1 would have hit a 404. Fixed.3. Clearer OAuth scope documentation
The README now has a single "Scope cheat sheet" table right under the OAuth client setup steps, including an "All-in (recommended for a single CI client)" row that lists every scope this action ever needs. Also called out that
*should be avoided in production. The duplicate "Required scopes" section near the bottom is replaced by a one-line cross-link.Test plan
uses:lines now reference `spicehq/spice-cloud-deploy-action`.Out of scope
@actions/core(pinned at v1 by Fix CI: Biome v2 schema + pin @actions/core to v1 #7; tracked as a follow-up).