Skip to content

Add repository field to package.json for npm provenance - #4

Merged
netanelgilad merged 3 commits into
mainfrom
netanelgilad-patch-1
Jan 5, 2026
Merged

Add repository field to package.json for npm provenance#4
netanelgilad merged 3 commits into
mainfrom
netanelgilad-patch-1

Conversation

@netanelgilad

@netanelgilad netanelgilad commented Jan 5, 2026

Copy link
Copy Markdown
Contributor

Adds the repository field to package.json to fix npm publish with provenance verification.

Changes

  • Added repository.type and repository.url fields pointing to https://github.com/base44/cli

Why

npm's sigstore provenance verification requires the repository.url in package.json to match the GitHub repository from which the package is being published.

@claude

claude Bot commented Jan 5, 2026

Copy link
Copy Markdown

Pull Request Review

Summary

This PR updates the CLI description by appending "2" to the description string. Based on the title "Test preview publish", this appears to be a test change to verify the publishing/preview workflow.

Code Quality & Best Practices

  • ✅ The change is syntactically correct
  • ⚠️ Issue: The change appears to be a test modification that should not be merged to main
    • The description change from 'Base44 CLI' to 'Base44 CLI 2' doesn't reflect an actual product/version change
    • This type of test content could confuse users about the actual CLI version

Inconsistencies Detected

  • ⚠️ package.json mismatch: The package.json description still reads "Base44 CLI - Unified interface for managing Base44 applications" (without the "2")
    • At src/cli/index.ts:13, the description is 'Base44 CLI 2 - ...'
    • At package.json:4, the description is 'Base44 CLI - ...'
    • These should be consistent if this is an intentional change

Potential Issues

  1. Purpose unclear: If this is truly a test PR, it should:

    • Be clearly labeled as "[TEST]" or "[DO NOT MERGE]"
    • Target a test branch, not main
    • Be closed without merging after verification
  2. Semantic meaning: If "2" represents a version:

    • Consider using semantic versioning in package.json instead
    • The CLI already has .version(getPackageVersion()) that reads from package.json
    • Adding version info to the description is redundant

Performance Considerations

✅ No performance impact - this is a string literal change only.

Security Concerns

✅ No security concerns identified.

Test Coverage

⚠️ Missing tests: The repository has no test files (no *.test.ts or *.spec.ts files found). Consider:

  • Adding unit tests for CLI commands
  • Integration tests for the overall CLI behavior
  • Tests to verify version/description output

Recommendation

If this is a test PR: Close without merging after verifying the preview/publish workflow works correctly.

If this is an intentional change:

  1. Clarify the purpose - what does "2" signify?
  2. Update package.json description to match
  3. Consider whether this information belongs in the description or should be handled through versioning

Please clarify the intent of this PR so I can provide more specific guidance.

@github-actions

github-actions Bot commented Jan 5, 2026

Copy link
Copy Markdown
Contributor

🚀 Package Preview Available!


Install this PR's preview build with npm:

npm i @base44-preview/cli@0.0.1-pr.4.36be66e

Prefer not to change any import paths? Install using npm alias so your code still imports base44:

npm i "base44@npm:@base44-preview/cli@0.0.1-pr.4.36be66e"

Or add it to your package.json dependencies:

{
  "dependencies": {
    "base44": "npm:@base44-preview/cli@0.0.1-pr.4.36be66e"
  }
}

Preview published to npm registry — try new features instantly!

@claude

claude Bot commented Jan 5, 2026

Copy link
Copy Markdown

Pull Request Review

Summary

This PR adds repository metadata to package.json and modifies the CLI description. The changes appear to be testing the preview publish workflow.


Code Quality & Best Practices

✅ Positive:

  • The repository field addition in package.json is correct and follows npm package.json standards
  • Proper formatting and structure maintained

⚠️ Concerns:

  • The description change in src/cli/index.ts:13 appears to be a test modification (adding "2" to "Base44 CLI")
  • This creates an inconsistency: package.json:4 still has the original description "Base44 CLI - Unified interface for managing Base44 applications", but the CLI now displays "Base44 CLI 2"

Recommendation:
If this change is intentional, the package.json description should be updated to match. If this is just for testing the preview publish, this modification should be reverted before merging.


Potential Bugs or Issues

Minor Inconsistency:

  • The CLI description and package.json description are now out of sync
  • Users might see different descriptions depending on where they look (npm registry vs. running base44 --help)

Security Concerns

✅ No security issues identified

  • Repository URL addition is benign and expected
  • No credentials, secrets, or vulnerable dependencies introduced

Test Coverage

⚠️ Testing Observations:

  • No tests currently exist in the repository (no *.test.ts or *.spec.ts files found)
  • These changes are low-risk and don't require immediate test coverage
  • However, consider adding tests for the CLI in the future, especially for:
    • Version display functionality
    • Command registration
    • Help text output

Performance Considerations

✅ No performance impact

  • Changes are purely metadata/configuration
  • No runtime performance implications

Recommendation

If this is for testing preview publish: Revert the description change in src/cli/index.ts before merging to production, keeping only the repository field addition.

If the description change is intentional: Update package.json:4 to match:

"description": "Base44 CLI 2 - Unified interface for managing Base44 applications",

The repository field addition is good and should be kept regardless.

@netanelgilad netanelgilad changed the title Test preview publish Add repository field to package.json for npm provenance Jan 5, 2026
@claude

claude Bot commented Jan 5, 2026

Copy link
Copy Markdown

Pull Request Review

Overview

This PR adds the repository field to package.json to enable npm provenance verification. The change is straightforward and addresses a specific npm publishing requirement.

✅ Code Quality & Best Practices

  • Clean implementation: The changes follow the standard npm package.json schema correctly
  • Proper placement: The repository field is placed logically after the license field, maintaining good organization
  • Minimal scope: The PR does exactly what it needs to without unnecessary changes

🔍 Potential Issues & Suggestions

1. Repository URL Format

The current URL format is missing the .git extension and protocol prefix. While npm accepts both formats, the more complete format is recommended:

Current:

"url": "https://github.com/base44/cli"

Recommended:

"url": "git+https://github.com/base44/cli.git"

This follows the npm package.json specification more precisely and ensures better compatibility with various tools.

2. Additional Repository Metadata (Optional)

Consider adding the directory field if this package is part of a monorepo (doesn't appear to be the case here, so this is just FYI for future reference).

🔒 Security Considerations

  • ✅ No security concerns - this is purely metadata
  • ✅ The repository URL correctly points to the intended GitHub repository
  • ✅ Provenance verification will improve supply chain security for consumers of this package

🚀 Performance Considerations

  • ✅ No performance impact - this is package metadata only

🧪 Test Coverage

  • ℹ️ No tests are needed for this change as it's purely declarative metadata
  • ℹ️ The real test will be whether npm publish with provenance succeeds in the CI/CD pipeline
  • 💡 Recommendation: After merging, verify that the provenance publishing works as expected

📝 Summary

Verdict: ✅ Approve with minor suggestion

This is a necessary and well-executed change. The only improvement would be to use the more complete URL format (git+https://.../.git), but the current format should work fine for provenance verification.

Action items before merge (optional):

  • Consider updating the URL to git+https://github.com/base44/cli.git
  • Verify provenance publishing works in a test publish or after merge

Great work addressing the npm provenance requirement! 🎉

@netanelgilad
netanelgilad marked this pull request as ready for review January 5, 2026 13:11
@netanelgilad
netanelgilad merged commit dd2ee80 into main Jan 5, 2026
3 checks passed
@netanelgilad
netanelgilad deleted the netanelgilad-patch-1 branch January 5, 2026 13:13
This was referenced Feb 5, 2026
Paveltarno added a commit that referenced this pull request Mar 9, 2026
- Rename loop variable `i` to `integration` (review #2)
- Use typed destructuring instead of Record<string, unknown> (review #3)
- Extract reusable formatYaml() utility with stripEmpty option (review #4)
- Use lodash isEmpty for null/empty stripping in YAML replacer
- Add formatYaml unit tests
- Update test assertions for integrationType key rename

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Paveltarno added a commit that referenced this pull request Mar 9, 2026
- Rename loop variable `i` to `integration` (review #2)
- Use typed destructuring instead of Record<string, unknown> (review #3)
- Extract reusable formatYaml() utility with stripEmpty option (review #4)
- Use lodash isEmpty for null/empty stripping in YAML replacer
- Add formatYaml unit tests
- Update test assertions for integrationType key rename

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Paveltarno added a commit that referenced this pull request Mar 9, 2026
* feat(connectors): add `connectors list-available` command

Add a new CLI command to discover all available integration types from
the Base44 catalog endpoint (GET /external-auth/available-integrations).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: address review — remove unused type, add min(1) to integration_type

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: align schema with apper commit 7556a23

- Wrapper key: `available_integrations` → `integrations`
- Remove `notes` and `usage_guide` fields (dropped from backend)
- Extract long template literal for readability

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* refactor: output full JSON for list-available

Print the complete integration data (including connection_config_fields)
as JSON to stdout so LLMs and scripts can consume all fields.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* feat: YAML-formatted output for list-available

- Add yaml dep for pretty-printing integration details
- Display each integration as YAML (type, description, config fields)
- Keep connection_config_fields in snake_case (no transform)
- Remove temp fake fields

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: restore camelCase transform for connection config fields

Keep consistent with codebase convention — convert snake_case to
camelCase at the Zod boundary for all fields.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* chore: add .claude/worktrees/ to gitignore

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: address review findings for list-available

- Use camelCase key `connectionConfigFields` in YAML output
- Strip null/undefined values from config fields before display
- Add comment explaining z.string() choice over IntegrationTypeSchema
- Add test for malformed API response (Zod validation)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* refactor: extract formatYaml helper, address review findings

- Rename loop variable `i` to `integration` (review #2)
- Use typed destructuring instead of Record<string, unknown> (review #3)
- Extract reusable formatYaml() utility with stripEmpty option (review #4)
- Use lodash isEmpty for null/empty stripping in YAML replacer
- Add formatYaml unit tests
- Update test assertions for integrationType key rename

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* chore: update bun.lock for yaml dependency

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: use main's root package.json for monorepo paths

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* style: fix Biome formatting in yaml tests

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant