-
-
Notifications
You must be signed in to change notification settings - Fork 1
feat(multipublish): add jsr auth token support and token environment key #224
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
8d8b474
b1ba558
cf485ac
574bcca
0b11229
82b9727
c35993e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@stephansama/multipublish": patch | ||
| --- | ||
|
|
||
| Updated jsr to have auth token. fail without error for unscoped packages | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ env: | |
| ZX_VERBOSE: true | ||
| DO_NOT_TRACK: 1 | ||
| NODE_AUTH_TOKEN: ${{secrets.NODE_AUTH_TOKEN}} | ||
| JSR_AUTH_TOKEN: ${{secrets.JSR_AUTH_TOKEN}} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Two observations:
🤖 Prompt for AI Agents |
||
| CODECOV_TOKEN: ${{secrets.CODECOV_TOKEN}} | ||
| GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}} | ||
| TURBO_TOKEN: ${{secrets.TURBO_TOKEN}} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -150,7 +150,8 @@ describe("publish", () => { | |
| ); | ||
| }); | ||
|
|
||
| it("should publish to jsr", async () => { | ||
| it("should publish to jsr without token", async () => { | ||
| vi.stubEnv("JSR_AUTH_TOKEN", ""); | ||
| vi.mocked(mocks.loadConfig).mockResolvedValue({ | ||
| config: { | ||
| exports: "index.ts", | ||
|
|
@@ -183,5 +184,42 @@ describe("publish", () => { | |
| { stdio: "inherit" }, | ||
| ); | ||
| }); | ||
|
|
||
| it("should publish to jsr with token", async () => { | ||
| const token = "test-token"; | ||
| vi.stubEnv("JSR_AUTH_TOKEN", token); | ||
|
|
||
| vi.mocked(mocks.loadConfig).mockResolvedValue({ | ||
| config: { | ||
| exports: "index.ts", | ||
| name: "@scope/pkg", | ||
| version: "1.0.0", | ||
| }, | ||
| filename: "/path/to/pkg/jsr.json", | ||
| }); | ||
|
|
||
| const pkg = { | ||
| dir: "/path/to/pkg", | ||
| packageJson: { | ||
| name: "@scope/pkg", | ||
| version: "1.0.0", | ||
| }, | ||
| relativeDir: "./pkg", | ||
| }; | ||
|
|
||
| const platform = "jsr"; | ||
|
|
||
| await publishPlatform(pkg, platform); | ||
|
|
||
| expect(mocks.writeFile).toHaveBeenCalledWith( | ||
| "/path/to/pkg/jsr.json", | ||
| expect.any(String), | ||
| ); | ||
|
|
||
| expect(mocks.execSync).toHaveBeenCalledWith( | ||
| `pnpm dlx jsr publish --allow-dirty --allow-slow-types --token ${token}`, | ||
| { stdio: "inherit" }, | ||
| ); | ||
| }); | ||
|
Comment on lines
+188
to
+223
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Token interpolated directly into shell command in The test at line 220 confirms
Consider switching to // In publish.ts (not in the test file)
const args = ["dlx", "jsr", "publish", "--allow-dirty", "--allow-slow-types"];
if (token) args.push("--token", token);
cp.execFileSync("pnpm", args, { stdio: "inherit" });The test assertion would then change to match 🤖 Prompt for AI Agents |
||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -75,15 +75,18 @@ export async function publishPlatform( | |
| } | ||
| } | ||
|
|
||
| const authToken = process.env[config.tokenEnvironmentKey]; | ||
|
|
||
| await util.chdir(pkg.dir, () => { | ||
| cp.execSync( | ||
| [ | ||
| jsrPublishCommand[packageManager], | ||
| "--allow-dirty", | ||
| isDryRun && "--dry-run", | ||
| config.allowSlowTypes && "--allow-slow-types", | ||
| isDryRun && "--dry-run", | ||
| authToken && `--token ${authToken}`, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Directly interpolating |
||
| ] | ||
| .filter((x) => x) | ||
| .filter((x): x is string => !!x) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Filter type guard inconsistency with the The JSR path uses a proper type guard ♻️ Proposed fix// Line 160 (npm path)
- .filter((x) => x)
+ .filter((x): x is string => !!x)🤖 Prompt for AI Agents |
||
| .join(" "), | ||
| { stdio: "inherit" }, | ||
| ); | ||
|
|
@@ -120,7 +123,9 @@ export async function publishPlatform( | |
|
|
||
| const scope = pkg.packageJson.name.split("/").at(0); | ||
| if (!scope?.startsWith("@")) { | ||
| throw new Error("scope must start with `@` symbol"); | ||
| return console.error( | ||
| "scope does not start with @ symbol. aborting.", | ||
| ); | ||
|
Comment on lines
+126
to
+128
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changing from throw new Error("scope must start with `@` symbol"); |
||
| } | ||
|
Comment on lines
+126
to
129
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
🐛 Proposed fix- return console.error(
- "scope does not start with @ symbol. aborting.",
- );
+ console.error("scope does not start with @ symbol. aborting.");
+ throw new Error("scope does not start with @ symbol. aborting.");Or, if silent continuation is intentional (best-effort multi-package publish), document this explicitly and ensure the caller accounts for no-op returns. 🤖 Prompt for AI Agents |
||
|
|
||
| const npmrcFile = | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,7 @@ export default defineConfig({ | |
| provider: "v8", | ||
| reporter: ["html", "json", "text"], | ||
| }, | ||
| outputFile: { junit: "./coverage/test-report.junit.xml" }, | ||
| outputFile: { junit: "./test-report.junit.xml" }, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# 1. Check if test-report.junit.xml at root is covered by .gitignore
echo "=== Checking .gitignore for test-report patterns ==="
if [ -f .gitignore ]; then
rg -n "test-report" .gitignore || echo "No test-report entry found in .gitignore"
else
echo ".gitignore not found"
fi
echo ""
echo "=== Checking for broad patterns that might cover root-level test files ==="
rg -n "^\*\.\(xml\|junit\)" .gitignore || echo "No broad XML/junit patterns found"
echo ""
echo "=== Full .gitignore content for context ==="
cat .gitignore
echo ""
echo "=== Checking CI workflow files for junit path references ==="
if [ -d .github/workflows ]; then
rg -n "test-report\|junit\|coverage" .github/workflows/ || echo "No junit/coverage references found in CI workflows"
else
echo ".github/workflows directory not found"
fiRepository: stephansama/packages Length of output: 850 🏁 Script executed: #!/bin/bash
# Check for other CI system config files
echo "=== Searching for CI configuration files ==="
fd -type f -name ".gitlab-ci.yml" -o -name "*.circleci*" -o -name "bitbucket-pipelines.yml" -o -name ".travis.yml" 2>/dev/null || echo "No other CI configs found via fd"
echo ""
echo "=== Searching for any 'test' or 'junit' references in .github/ recursively ==="
if [ -d .github ]; then
find .github -type f | head -20
echo ""
rg -i "test|junit|artifact" .github/ || echo "No artifact/test references in .github"
else
echo ".github directory not found"
fi
echo ""
echo "=== Checking if .github/workflows exists and listing files ==="
if [ -d .github/workflows ]; then
ls -la .github/workflows/
else
echo ".github/workflows not found"
fi
echo ""
echo "=== Broader search for 'test-report' in entire repo (excluding node_modules) ==="
rg -i "test-report\|junit\.xml" --glob='!node_modules' . 2>/dev/null | head -20Repository: stephansama/packages Length of output: 1309 Update The junit report path change from
Without these changes, the junit report will be committed to the repo and the CI test upload will fail. 🤖 Prompt for AI Agents |
||
| projects: ["./core/*"], | ||
| reporters: ["default", "junit"], | ||
| }, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: stephansama/packages
Length of output: 4830
🏁 Script executed:
Repository: stephansama/packages
Length of output: 1711
🏁 Script executed:
Repository: stephansama/packages
Length of output: 1019
🏁 Script executed:
Repository: stephansama/packages
Length of output: 130
🏁 Script executed:
Repository: stephansama/packages
Length of output: 3811
🏁 Script executed:
Repository: stephansama/packages
Length of output: 1029
🏁 Script executed:
Repository: stephansama/packages
Length of output: 867
🏁 Script executed:
Repository: stephansama/packages
Length of output: 1613
🏁 Script executed:
Repository: stephansama/packages
Length of output: 1521
🏁 Script executed:
Repository: stephansama/packages
Length of output: 726
Clarify the changelog description; silent-failure behavior for unscoped packages is confirmed in the NPM publish path.
The phrase "fail without error for unscoped packages" is grammatically awkward and ambiguous. More importantly, this silent-failure behavior is confirmed in the codebase at the NPM publish handler (lines 125–129 of
publish.ts): when a package is unscoped,console.error()logs a diagnostic but the function returns without throwing, allowing the CI/workflow to continue appearing successful.Reword for clarity and specify which platform(s) this applies to:
📝 Committable suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 5-5: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 Prompt for AI Agents