Skip to content

Conversation

@designcode
Copy link
Collaborator

@designcode designcode commented Jan 22, 2026

Note

Introduces access key management in the IAM SDK and refactors release to per-package semantic-release with a scripted dependency order.

  • Adds listAccessKeys, getAccessKey, createAccessKey, removeAccessKey, and assignBucketRoles with form-encoded requests; exports from packages/iam/src/index.ts
  • Updates shared http-client to sign FormData/URLSearchParams bodies for AWS SigV4
  • Replaces monorepo release with per-package configs: new release.config.base.cjs, package-specific release.config.cjs, and scripts/release.sh; updates package.json scripts and CI workflows (pr.yaml, release.yaml)
  • ESLint now ignores *.cjs files

Written by Cursor Bugbot for commit 5c2fa07. This will update automatically on new commits. Configure here.

@greptile-apps
Copy link

greptile-apps bot commented Jan 22, 2026

Greptile Overview

Greptile Summary

This PR refactors the CI release process by removing multi-semantic-release in favor of a custom bash script that releases packages sequentially in dependency order.

Major Issues:

  • PR title "feat(iam): access key operations" doesn't match actual changes (commit: "chore(ci): remove multi-semantic-release package")
  • NPM_TOKEN environment variable removed from workflow but @semantic-release/npm plugin still needs it for publishing
  • Hardcoded branch iam/access-keys in release config should be removed before merging to main

Changes:

  • Replaced multi-semantic-release with custom shell script for sequential package releases
  • Created shared release.config.base.cjs and individual package configs
  • Updated semantic-release plugin versions
  • Added *.cjs to ESLint ignore patterns

Confidence Score: 1/5

  • This PR will break NPM publishing in CI and contains configuration issues
  • Two critical issues will prevent successful releases: missing NPM_TOKEN environment variable breaks package publishing, and hardcoded 'iam/access-keys' branch reference is inappropriate for main branch. Additionally, PR title completely misrepresents the changes.
  • Pay close attention to .github/workflows/release.yaml (missing NPM authentication) and release.config.base.cjs (hardcoded branch reference)

Important Files Changed

Filename Overview
.github/workflows/release.yaml Removed NPM_TOKEN environment variable from release workflow, which may break NPM publishing
release.config.base.cjs Added base semantic-release config with hardcoded 'iam/access-keys' branch that should be removed before merging to main

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

Comment on lines 52 to +54
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
NPM_TOKEN: ${{ secrets.NPM_TOKEN }}
run: npm run release
run: ./scripts/release.sh
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NPM_TOKEN environment variable was removed but @semantic-release/npm plugin still requires authentication to publish packages to npm. The workflow will fail when attempting to publish.

Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/release.yaml
Line: 52:54

Comment:
NPM_TOKEN environment variable was removed but @semantic-release/npm plugin still requires authentication to publish packages to npm. The workflow will fail when attempting to publish.

How can I resolve this? If you propose a fix, please make it concise.

// Extract scope from package name: '@tigrisdata/storage' -> 'storage'
const scope = packageName.replace('@tigrisdata/', '');

return {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The branch 'iam/access-keys' is hardcoded in the branches array. This temporary branch reference should be removed before merging to main, or it will allow releases from that branch in perpetuity.

Suggested change
return {
branches: ['main', { name: 'next', prerelease: true }],
Prompt To Fix With AI
This is a comment left during a code review.
Path: release.config.base.cjs
Line: 9:9

Comment:
The branch 'iam/access-keys' is hardcoded in the branches array. This temporary branch reference should be removed before merging to main, or it will allow releases from that branch in perpetuity.

```suggestion
    branches: ['main', { name: 'next', prerelease: true }],
```

How can I resolve this? If you propose a fix, please make it concise.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

role: role.role,
})),
},
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accessing array index without bounds checking causes crash

Medium Severity

The getAccessKey function directly accesses response.data.Keys[0] without verifying the array contains elements. If the API returns an empty Keys array (e.g., when the access key ID doesn't exist), this will throw a runtime error when accessing properties like .access_key_id on undefined.

Fix in Cursor Fix in Web


export type CreateAccessKeyResponse = {
accessKeyId: string;
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exported type doesn't match function return type

Low Severity

The CreateAccessKeyResponse type is exported but never used and doesn't match the actual return type. The type contains only { accessKeyId: string }, but createAccessKey returns TigrisIAMResponse<AccessKey, Error> which includes id, secret, name, createdAt, status, and roles. Consumers importing this type would be misled about what the function returns.

Additional Locations (1)

Fix in Cursor Fix in Web

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.

2 participants