👽️ Update schemas #39
Conversation
NatoBoram
requested review from
averyjennings and
sleep-404
and removed request for
a team
📝 WalkthroughWalkthroughThe changes integrate various repository updates. The .gitattributes file now marks specific files as linguist-generated. A GitHub Actions bot key was removed from .github/authorized_keys. Multiple GitHub workflow files have been modified to update event triggers, add a new “fix” job, remove signing steps, and streamline the version patch process via new patch and commit steps. In addition, package.json updates include revised test scripts and a TypeScript dependency upgrade. API endpoints for GPG key management were added, and new compiler options were introduced in tsconfig.json and tsconfig.eslint.json. Changes
Suggested labels
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
tsconfig.json (1)
31-35: New Compiler Option Added: "erasableSyntaxOnly"The
"erasableSyntaxOnly": trueoption has been added to thecompilerOptionssection. Please verify that this experimental option is supported in TypeScript version"5.8.1-rc"and that its intended effect on the emitted code is well documented for team members..github/workflows/pnpm-publish.yaml (1)
47-49: Updated Release Upload StepThe SSH signing process has been removed and replaced with a simplified release upload using
gh release upload. Ensure that this revised process meets your security requirements and that the release artefacts are correctly identified and uploaded as intended.package.json (2)
42-43: Revised Test ScriptsThe
"test": "vitest run"command now explicitly runs the tests, and the addition of"test:watch": "vitest"supports continuous testing during development. Please confirm that the CI/CD pipeline and developer documentation are updated to reflect these script changes.
68-68: TypeScript Version UpgradeTypeScript has been updated to
"5.8.1-rc", a release candidate version. Please ensure that this upgrade does not introduce any unexpected issues in production, and communicate the potential experimental nature of this version to the team..github/workflows/node.js.yaml (1)
57-119: New "fix" Job ImplementationA new
fixjob has been introduced to automatically address lockfile issues, formatting discrepancies, and lint errors when tests fail. Ensure that theifcondition accurately captures the failure states and that the consecutive commit steps integrate seamlessly with your Git workflow..github/workflows/pnpm-version-patch.yaml (1)
32-39: Enhanced Release CreationThe final step pushes changes and uses
gh release createto generate a release with appropriate notes. Make sure that the environment variablesCOMMITandVERSIONare correctly passed and that the GitHub CLI is available in the runner environment.src/cloud/openapi/openapi-typescript.ts (1)
15636-15868:❓ Verification inconclusive
Consider clarifying unauthenticated vs. unauthorized responses.
While 403 codes are provided for authorization failures, it may be helpful to also define a 401 response if the request lacks authentication credentials. Everything else about these new GPG key endpoints looks great.Below is a script to check for usage of “401” in the repository, to see how it is handled:
🏁 Script executed:
#!/bin/bash rg -A 3 '"401"'Length of output: 89047
Clarify Unauthenticated vs. Unauthorized Responses
Great work on these new GPG key endpoints overall. One suggestion for improvement: consider adding an explicit 401 response for cases where a request is unauthenticated, in addition to the 403 responses that indicate insufficient authorisation. A quick search shows that our other Swagger definitions (e.g. in
src/cloud/openapi/swagger.v3.jsonandsrc/server/openapi/swagger.v3.json) already make this distinction. Please review whether adding a 401 response here would improve consistency and error clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
.gitattributes(1 hunks).github/authorized_keys(0 hunks).github/workflows/node.js.yaml(3 hunks).github/workflows/pnpm-publish.yaml(1 hunks).github/workflows/pnpm-version-patch.yaml(1 hunks)package.json(2 hunks)src/cloud/openapi/openapi-typescript.ts(14 hunks)tsconfig.json(1 hunks)
💤 Files with no reviewable changes (1)
- .github/authorized_keys
✅ Files skipped from review due to trivial changes (1)
- .gitattributes
🔇 Additional comments (17)
.github/workflows/node.js.yaml (2)
3-12: Updated Event TriggersThe workflow has been updated to include a new
merge_grouptrigger alongside the existingpull_requestandpushevents. Confirm that these triggers align with your intended CI events and that no critical triggers have been inadvertently omitted.
47-48: Corrected Test CommandThe test step now correctly invokes
pnpm run testrather than the previously erroneous command. Please double-check that this change properly executes your test suite across all environments..github/workflows/pnpm-version-patch.yaml (2)
22-26: Version Patch Step AddedThe new
patchstep usespnpm version patch --no-git-tag-versionand writes the resulting version to the GitHub output. Verify that this approach meets your versioning strategy and that the version output is captured correctly for subsequent steps.
27-31: Commit Automation Step IncludedThe workflow now employs
qoomon/actions--create-commit@v1to commit the version bump automatically. Please confirm that this action is configured correctly and that it properly incorporates all necessary commit metadata.src/cloud/openapi/openapi-typescript.ts (13)
2618-2637: Documentation references appear consistent.
These lines provide clear instructions for associating commit statuses with pull requests. No concerns observed.
5516-5520: Status codes are comprehensive for issue deletion.
No issues found with using 204 for success and 403 for lack of authorization.
8024-8035: Pipelines listing endpoint is properly documented.
The query parameters appear to be defined elsewhere, and everything here looks fine.
19109-19115: Commits schema looks consistent.
The optional committer aligns well with current design patterns.
19458-19466: New committer object usage is appropriate.
No issues found in this definition.
19749-19787: Confirm that only public keys are stored.
Since we are referencing a GPG public key, ensure no private key data is accidentally included.
20478-20501: Paginated GPG user keys schema is well-defined.
Coverage of paging fields (next, page, pagelen, etc.) is consistent with standard pagination.
23291-23291: Type export “SchemaCommitter” is properly referenced.
No issues to report.
23323-23323: Type export “SchemaGpgAccountKey” is introduced correctly.
Straightforward addition with no concerns.
23363-23364: Type export “SchemaPaginatedGpgUserKeys” is valid.
No concerns; pagination schema is referenced correctly.
24380-24422: Query parameters for repository pipelines are clearly documented.
All relevant filters appear accounted for.
25469-25473: 307 redirect for long-term storage log retrieval looks standard.
No issues raised.
25515-25519: Another 307 redirect is consistently applied here.
This is aligned with the same approach in the prior snippet.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tsconfig.eslint.json (1)
34-34: New Property Integration: Verify 'erasableSyntaxOnly' Setting
The addition of"erasableSyntaxOnly": trueon line 34 is consistent with the corresponding change intsconfig.jsonand aligns with the updated schema. Please verify that this new configuration is supported by your ESLint TypeScript tooling and does not cause unintended effects despite the static analysis tool's false positives.🧰 Tools
🪛 Biome (1.9.4)
[error] 34-34: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 34-34: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 34-34: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 34-34: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
tests/env.ts (1)
7-7: Naming clarity
Using the same identifier (NodeEnv) for both the constant object and the type can create minor confusion.Consider renaming the type for clarity:
- type NodeEnv = (typeof NodeEnv)[keyof typeof NodeEnv] + type NodeEnvironment = (typeof NodeEnv)[keyof typeof NodeEnv]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.env(1 hunks)package.json(2 hunks)tests/cloud/repositories.test.ts(1 hunks)tests/env.ts(2 hunks)tests/server/projects.test.ts(1 hunks)tests/server/repositories.test.ts(1 hunks)tsconfig.eslint.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .env
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
🪛 Biome (1.9.4)
tsconfig.eslint.json
[error] 34-34: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 34-34: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 34-34: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 34-34: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
🔇 Additional comments (15)
tests/cloud/repositories.test.ts (2)
2-2: Import of environment variable is straightforward
No issues identified. This aligns well with your environment-based test management approach.
5-5: Conditional skipping logic is properly implemented
Usingtest.skipIf(SKIP_BITBUCKET_CLOUD)is a clean way to manage tests that depend on Bitbucket Cloud availability.tests/server/projects.test.ts (2)
5-5: Import usage is consistent
IncorporatingSKIP_BITBUCKET_SERVERfollows your environment-driven strategy, ensuring better test handling.
9-9: Environment-based skipping for 'Projects'
Conditionally skipping the entire suite whenSKIP_BITBUCKET_SERVERis enabled helps prevent spurious failures in unsupported environments.tests/server/repositories.test.ts (2)
5-5: Environment variable import
Adding theSKIP_BITBUCKET_SERVERimport maintains a consistent pattern for skipping server-specific tests.
9-70: describe.skipIf usage with concurrency settings
Utilizingdescribe.skipIf(SKIP_BITBUCKET_SERVER)and specifying{ concurrent: false, sequential: true }is a sensible approach for controlling test execution order and preventing tests from running in unsupported contexts. The integration tests for Create/Get/Delete functionalities appear correct, with no evident logical or structural concerns.tests/env.ts (9)
1-1: Switched to @natoboram/load_env
This library-based environment loading provides a clean, typed interface, simplifying configuration handling.
9-10: Type guard isNodeEnv
Validating the environment value againstObject.values(NodeEnv)is straightforward and effective.
13-15: Graceful fallback logic
ReturningNodeEnv.developmentifvalueis invalid is a protective measure ensuring default behaviour.
18-18: Const object definition
Usingas constlocks in the valid Node environment strings, enhancing type safety.
28-28: Assigned parsed environment
Directly capturing the parsed environment variables encourages consistency across the test suite.
30-30: Wrapped NODE_ENV with toNodeEnv
Ensures only valid environment values, defaulting to development if unrecognised.
37-37: Minor structural change
Likely a whitespace or minor syntax adjustment. No functional issues detected.
47-47: SKIP_BITBUCKET_CLOUD introduced
Defining a boolean environment variable fosters controlled skipping of Cloud-based integration tests.
49-55: SKIP_BITBUCKET_SERVER documentation
Providing inline commentary clarifies why Data Center tests might be skipped. This practice helps maintain transparency for future maintainers.
The schema changes add some API for managing a user's GPG key in Bitbucket Cloud.
tsx, the future is now!Summary by CodeRabbit
New Features
Chores