Skip to content

chore: update deno to v2.2.4 #923

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

Merged
merged 46 commits into from
Jun 8, 2025
Merged

chore: update deno to v2.2.4 #923

merged 46 commits into from
Jun 8, 2025

Conversation

Yohe-Am
Copy link
Contributor

@Yohe-Am Yohe-Am commented Nov 16, 2024

  • Updates deno to 2.2.4
  • Updates rust to 1.85.0

Summary by CodeRabbit

  • New Features

    • Added a utility for creating AWS S3 clients with MD5 support.
    • Added a new health check ("ping") action for TypeGate runtimes.
  • Bug Fixes

    • Improved artifact and S3 file upload/download reliability by using presigned URLs and HTTP requests.
    • Enhanced error handling across multiple services and runtimes for more consistent and informative error reporting.
  • Refactor

    • Updated Rust and Deno toolchain versions for improved compatibility and performance.
    • Streamlined permissions and error handling in various runtimes and services.
    • Switched to type-only imports in TypeScript for better type safety and reduced runtime overhead.
  • Chores

    • Updated dependencies and Docker configurations.
    • Improved CI/CD workflows for manual tool installation and environment setup.
    • Disabled or moved some end-to-end tests for future re-enablement.

@Yohe-Am Yohe-Am requested a review from a team November 16, 2024 22:47
Natoandro
Natoandro previously approved these changes Nov 17, 2024
Natoandro
Natoandro previously approved these changes Dec 9, 2024
@Yohe-Am Yohe-Am changed the title chore: update deno to v2.0.6 chore: update deno to v2.1.2 Dec 16, 2024
Copy link
Contributor

coderabbitai bot commented Jan 2, 2025

📝 Walkthrough

Walkthrough

This set of changes introduces a new unified error type (OpErr) for Rust hostcall operations in the Typegate engine, refactors all Rust hostcall functions to return this error type, and updates error mapping throughout the Rust codebase. It also updates the TypeScript bindings to expect the new error signatures, and adds explicit any typing to error catches across the TypeScript codebase. Additional changes include dependency version updates, Dockerfile and workflow improvements, and widespread adoption of type-only imports in TypeScript for clarity and bundling efficiency.

Changes

File(s) / Path(s) Change Summary
src/typegate/engine/src/error.rs Introduced new OpErr error type implementing deno_error::JsErrorClass, with conversion, display, and deref traits for unified error handling in hostcalls.
src/typegate/engine/src/py_validation.rs, src/typegate/engine/src/runtimes/grpc.rs, ... Refactored all Rust hostcall functions to return Result<T, OpErr> and mapped errors to OpErr using map_err(OpErr::map()).
src/typegate/engine/src/typegraph.rs, src/typegate/engine/src/runtimes/prisma.rs, ... Updated hostcall return types and error mapping to use OpErr for all Prisma, Temporal, Substantial, Wasm, and validation operations.
src/typegate/engine/src/runtimes/wit_wire.rs Updated error enums to derive deno_error::JsError, added new error variants, and adjusted function signatures.
src/typegate/engine/src/ext.rs, src/typegate/engine/src/lib.rs Updated test harnesses and engine launchers to use new permissions and error types.
src/typegate/engine/bindings.ts, src/typegate/src/services/artifact_service.ts, ... Updated TypeScript bindings and service layers to expect errors as any, reflecting new error propagation from Rust hostcalls.
src/typegate/engine/runtime.d.ts, src/typegate/engine/runtime.js Changed Meta from a global to an explicit export, updated global assignment, and adjusted usage accordingly.
src/typegate/src/utils/mod.ts Added createS3ClientWithMD5 utility for S3 client instantiation with buffer size; commented-out advanced MD5 checksum middleware.
src/typegate/src/runtimes/s3.ts, src/typegate/src/typegate/artifacts/shared.ts Switched S3 upload/fetch operations to use presigned URLs and HTTP fetch, improving compatibility and error handling.
Cargo.toml, src/typegate/engine/Cargo.toml, ... Updated dependencies: Deno, Rust toolchains, and related crates. Added deno_error as a workspace dependency.
tools/Dockerfile, tools/consts.ts, rust-toolchain.toml Updated Rust and Deno versions to latest supported, removed/updated related install scripts and targets.
tools/compose/compose.base.yml, tools/compose/compose.subredis.yml Updated Docker Compose files: added persistent Minio volume, updated Minio image, and user directives.
.github/workflows/*.yml, tools/installs.ts, .ghjk/lock.json Updated CI workflows and install scripts to use new Rust/Deno versions and manual install steps for ghjk.
src/typegate/src/runtimes/substantial.ts, src/typegate/src/runtimes/python.ts, ... Widespread adoption of TypeScript type-only imports for improved clarity and bundling.
src/typegate/src/typegate/hooks.ts, src/typegate/src/typegate/mod.ts Added typegate parameter to push hooks, updated handler signatures, and improved error variable naming.
tests/e2e/self_deploy/self_deploy_test.tstests/e2e/self_deploy/self_deploy_test.ts.disable Disabled and renamed self-deploy E2E tests, consistent with pending implementation.
tests/e2e/self_deploy/self_deploy_bad_cred_test.tstests/e2e/self_deploy/self_deploy_bad_cred_test.ts.disabled Disabled and renamed bad credential E2E test.
src/typegate/engine/00_runtime.js, src/typegate/engine/runtime.d.ts, ... Changed global assignment and export of Meta, updated all consumers to use explicit import/export.
Many .ts and .rs files (services, runtimes, tests, etc.) Added explicit any typing to error catch blocks in TypeScript; updated error handling and variable naming for clarity and robustness.
src/typegate/engine/src/runtimes/wasm/conversion.rs, src/typegate/engine/src/runtimes/substantial.rs, ... Updated imports and lifetimes for consistency and clarity in Rust codebase, with no logic changes.
src/typegraph/schema/src/validator/types.rs, src/typegraph/schema/src/visitor.rs, ... Simplified Rust lifetime annotations for trait implementations and struct methods.

Sequence Diagram(s)

sequenceDiagram
    participant TS as TypeScript Layer
    participant Rust as Rust Hostcall
    participant OpErr as OpErr Error Type

    TS->>Rust: Call hostcall (e.g., op_prisma_query)
    Rust->>Rust: Execute operation
    alt Success
        Rust-->>TS: Return Result<T, OpErr> (Ok)
    else Error
        Rust->>OpErr: Map error to OpErr
        Rust-->>TS: Return Result<T, OpErr> (Err(OpErr))
    end
    TS->>TS: catch (err: any) { ... }
Loading

Possibly related PRs

  • metatypedev/metatype#937: Introduces new policy states ("ALLOW", "DENY", "PASS") and refactors policy handling, directly related to policy system changes in this PR.
  • metatypedev/metatype#982: Implements and refactors hostcall transport mechanisms across FDKs, directly related to hostcall error handling and transport changes in this PR.
  • metatypedev/metatype#962: Implements worker pooling and resource management in worker manager classes, which may interact with new error propagation and hostcall handling.

Suggested reviewers

  • Natoandro
✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (49)
src/mt_deno/src/lib.rs (2)

246-252: **Performing type checks **

Calling check_specifiers for type validation is an excellent step. Consider whether partial failures or concurrency might need explicit handling or more granular error reporting.


272-273: **Passing desc_parser to test executor **

Passing the desc_parser is clear. As a minor suggestion, consider logging or handling parser configuration errors for enhanced debugging, if applicable.

tests/schema_validation/type_comparison_test.ts (1)

10-10: **Use of catch (err: any) **

Specifying any clarifies the shape of the caught exception. If feasible, consider a narrower type or a custom error interface to improve type safety and debugging clarity.

src/typegate/src/services/auth/protocols/internal.ts (1)

52-52: Consider using a more specific type than any in the catch block.

While this approach works, using unknown or Error (when certain) can improve type safety. Consider narrowing or casting the error instead of relying on the any type.

src/typegate/src/services/auth/protocols/jwt.ts (1)

60-67: Avoid using any for error handling.

Using unknown or a more specific type can help catch issues at compile time. It's often best to narrow or cast the error.

tests/utils/s3.ts (2)

40-40: Consider using a more specific type instead of any.
While catching errors as any is quick, it may obscure potential type checks. Consider using a more specific error type or a type guard if feasible.


58-58: Replicate the approach from the previous catch block.
Same feedback applies here. If you can narrow down the type of raised exceptions, you could make your error handling more robust.

tools/list-duplicates.ts (2)

41-41: Safe casting approach.
Casting type.injection as any is understandable here. If you have a stable interface, consider using a stricter type.


55-55: Same reasoning about any.
To prevent losing type information, confirm that type.injection has a known shape and, if possible, declare a dedicated interface for it.

src/typegate/src/services/graphql_service.ts (1)

123-145: Ensure comprehensive logging and consistent error responses.

All error branches now log and return a response with err.message. Ensure that sensitive details are never leaked into logs. Otherwise, this is a clear, centralized approach to error handling.

src/xtask/src/deno.rs (1)

54-79: Refine commented-out allowed commands.

The large block of commented commands can make maintenance cumbersome. If you no longer need them, consider removing them permanently.

src/typegate/src/services/artifact_service.ts (1)

37-37: Consider using unknown instead of any.

While specifying a type for the caught error is an improvement over not specifying one at all, using unknown in place of any can enhance type safety and reduce the risk of runtime type errors.

src/typegate/engine/src/ext.rs (1)

142-146: Remove or clarify commented code.

The commented-out permissions code might be confusing to future maintainers unless there is an intention to restore or replace it. Consider removing it entirely or adding an explanatory comment describing why it remains in the file and when it might be reintroduced.

Also applies to: 148-148

src/typegate/src/services/rest_service.ts (1)

Line range hint 92-129: Consistent renaming of error variables is clear and precise.

Renaming e to err promotes clarity in error handling. However, consider using unknown instead of any for stronger type safety in TypeScript. Beyond this, the multiple checks for BaseError, ResolverError, and BadContext remain logically sound, ensuring appropriate HTTP response codes and JSON responses for each case.

src/typegate/src/libs/jsonpath.ts (1)

203-203: Explicit error typing strengthens maintainability.

Replacing an untyped catch variable with catch (e: any) clarifies the nature of the error, though using unknown could provide even better type safety and help guard against unintentional usage of error properties.

src/typegate/engine/src/lib.rs (1)

Line range hint 165-191: Consider removing commented-out default allow_import entries or converting them into configuration.

Having large commented-out segments may cause confusion. If you anticipate using these default hosts, consider making them configurable or removing them altogether to keep your codebase clean and maintainable.

src/typegate/src/runtimes/prisma/hooks/run_migrations.ts (1)

110-110: Use a more precise error type to improve type-checking.

Catching errors as any sacrifices type safety. Consider using unknown and then narrowing or wrapping the error, ensuring you capture exact information about failures.

tests/e2e/cli/deploy_test.ts (2)

86-86: Consider narrowing the error type in the catch block.

Catching exceptions as any is generally discouraged; using unknown and then narrowing or type-guarding may improve error handling reliability.


142-142: Improve error typing.

Again, consider using unknown to avoid losing compile-time checks and enforce explicit type narrowing. This helps preserve important error details when handling edge cases.

src/typegate/engine/bindings.ts (21)

59-59: Use more specific typing for error handling.

Catching errors as any forfeits type safety. Prefer unknown and then perform runtime checks or cast to a known error interface.


84-84: Enhance error type usage in catch.

Using any can hide actual error structures. Switch to unknown and handle possible error cases explicitly.


108-108: Encourage explicit error typing.

Relying on any leads to potential maintainability issues. If possible, replace with unknown and narrow the type.


127-127: Improve error typing for better clarity.

Switch from any to unknown or a known error type to leverage TypeScript’s type checks.


149-149: Refine error handling approach.

Catching an any type can conceal important details; prefer unknown plus a type guard or instance check.


171-171: Strengthen type safety in error handling.

Change any to unknown for stricter compile-time checks, reducing potential runtime errors.


190-190: Favor narrower error types.

Catching errors as any may hide critical type specifics, consider using unknown.


211-211: Adopt more explicit error type.

Maintain code clarity by dropping any in favor of unknown or a specialized error type.


234-234: Increase error-type clarity.

Use unknown to encourage explicit checking or transformation of caught errors.


253-253: Use explicit error handling strategy.

Replace any with unknown, forcing callers to handle error details properly.


275-275: Refine catch error typing.

A narrower type than any helps ensure that error properties are validated before use.


297-302: Leverage rigorous error typing.

Both catch blocks here use any. Reviewing them for unknown usage can improve clarity and safety.


339-339: Favor safer error typing.

Handling errors as unknown encourages robust validation and prevents accidental reliance on undefined properties.


362-362: Improve error stability with narrower types.

Using unknown ensures that reading properties on the error is done in a controlled manner.


388-388: Enhance reliability with explicit error typing.

Switching from any to unknown can help avoid unexpected runtime failures.


412-412: Strengthen error safety.

unknown is preferable over any to guarantee proper type checks for errors.


431-431: Encourage a typed catch block.

Use unknown in place of any so that error checks become more deliberate.


454-454: Refactor catch clause for better type checks.

Rely on a properly typed approach rather than any, ensuring strong error validation.


473-473: Favor narrower error type to maintain clarity.

By removing any, you reduce accidental assumptions about error structure or fields.


496-496: Use typed errors for clarity & safety.

Switch from any to unknown to align with TypeScript best practices for error handling.


516-516: Adopt a narrower type in catch for consistency.

Maintain consistent error handling throughout the codebase by ditching any for unknown.

src/typegate/src/typegate/mod.ts (1)

215-215: Enhance unknown error messaging
Using err.toString() is acceptable for unknown errors, but to improve debuggability, consider adding more metadata (e.g., stack trace) to the error message or logs.

tests/utils/test.ts (1)

357-357: Maintain consistent error-typing in test assertions
Typing the caught error as any is fine for tests, but if possible, consider more specific error types to gain type safety and avoid masking real issues.

src/typegate/src/engine/planner/policies.ts (1)

28-28: Clarify type usage for PolicyResolverOutput
Switching from (unknown & {}) to (unknown & any) removes some of the safety introduced by stricter types. Consider whether any is required or if there is a more precise type that captures the intended usage.

.github/workflows/publish-website.yml (2)

32-32: Trailing spaces found
Remove the trailing spaces to avoid triggering linter warnings.

-      
+
🧰 Tools
🪛 yamllint (1.35.1)

[error] 32-3232: trailing spaces

(trailing-spaces)


34-34: Quote the GHJK_VERSION variable to prevent word splitting
ShellCheck warns about potential issues if the $GHJK_VERSION variable contains spaces. Even if unlikely, quoting is considered best practice.

- - run: curl -fsSL https://raw.github.com/metatypedev/ghjk/$GHJK_VERSION/install.sh | bash
+ - run: curl -fsSL "https://raw.github.com/metatypedev/ghjk/$GHJK_VERSION/install.sh" | bash
🧰 Tools
🪛 actionlint (1.7.4)

34-3434: shellcheck reported issue in this script: SC2086:info:1:52: Double quote to prevent globbing and word splitting

(shellcheck)

Cargo.toml (1)

62-62: Note about deprecated dependency

The comment indicates that serde_yaml is deprecated. Consider tracking this as a future improvement.

.github/workflows/tests.yml (1)

173-173: Remove trailing whitespace

There are trailing spaces on lines 173 and 284.

Also applies to: 284-284

🧰 Tools
🪛 yamllint (1.35.1)

[error] 173-173173: trailing spaces

(trailing-spaces)

.github/workflows/release.yml (1)

38-42: Consider using a composite action to reduce code duplication.

The ghjk installation steps are repeated in multiple jobs. Consider creating a composite action to encapsulate these steps for better maintainability.

Example composite action:

# .github/actions/setup-ghjk/action.yml
name: 'Setup GHJK'
description: 'Installs GHJK'
runs:
  using: "composite"
  steps:
    - run: curl -fsSL "https://raw.github.com/metatypedev/ghjk/${GHJK_VERSION}/install.sh" | bash
      shell: bash
    - run: echo "$HOME/.local/bin" >> "$GITHUB_PATH"
      shell: bash
    - run: echo "BASH_ENV=$HOME/.local/share/ghjk/env.sh" >> "$GITHUB_ENV"
      shell: bash
    - run: ghjk envs cook
      shell: bash

Usage:

steps:
  - uses: ./.github/actions/setup-ghjk

Also applies to: 106-109, 186-190, 326-329

🧰 Tools
🪛 actionlint (1.7.4)

39-3939: shellcheck reported issue in this script: SC2086:info:1:52: Double quote to prevent globbing and word splitting

(shellcheck)

src/typegate/src/engine/planner/args.ts (1)

802-802: Consider using a more specific error type than any.

While adding a type annotation is good, using any reduces type safety. Consider using unknown for better type safety, or if possible, a more specific error type.

-          } catch (e: any) {
+          } catch (e: unknown) {
             const msg = e.message;

If using unknown, you'll need to add a type guard:

} catch (e: unknown) {
  const msg = e instanceof Error ? e.message : String(e);
  throw new BadContext("Error while querying context: " + msg);
}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 87b70e5 and 7d2be27.

⛔ Files ignored due to path filters (2)
  • Cargo.lock is excluded by !**/*.lock
  • deno.lock is excluded by !**/*.lock
📒 Files selected for processing (51)
  • .ghjk/lock.json (13 hunks)
  • .github/workflows/publish-website.yml (1 hunks)
  • .github/workflows/release.yml (4 hunks)
  • .github/workflows/tests.yml (5 hunks)
  • Cargo.toml (3 hunks)
  • deno.jsonc (1 hunks)
  • ghjk.ts (0 hunks)
  • import_map.json (1 hunks)
  • rust-toolchain.toml (1 hunks)
  • src/meta-cli/src/cli/typegate.rs (1 hunks)
  • src/metagen-client-rs/src/graphql.rs (2 hunks)
  • src/mt_deno/src/lib.rs (6 hunks)
  • src/substantial/src/backends/redis.rs (1 hunks)
  • src/typegate/engine/bindings.ts (21 hunks)
  • src/typegate/engine/src/ext.rs (1 hunks)
  • src/typegate/engine/src/lib.rs (3 hunks)
  • src/typegate/src/engine/planner/args.ts (1 hunks)
  • src/typegate/src/engine/planner/policies.ts (1 hunks)
  • src/typegate/src/libs/jsonpath.ts (1 hunks)
  • src/typegate/src/runtimes/deno/deno.ts (1 hunks)
  • src/typegate/src/runtimes/deno/deno_messenger.ts (0 hunks)
  • src/typegate/src/runtimes/deno/hooks/mod.ts (2 hunks)
  • src/typegate/src/runtimes/prisma/hooks/run_migrations.ts (1 hunks)
  • src/typegate/src/runtimes/python/hooks/mod.ts (1 hunks)
  • src/typegate/src/runtimes/typegate.ts (1 hunks)
  • src/typegate/src/services/artifact_service.ts (2 hunks)
  • src/typegate/src/services/auth/mod.ts (1 hunks)
  • src/typegate/src/services/auth/protocols/internal.ts (1 hunks)
  • src/typegate/src/services/auth/protocols/jwt.ts (1 hunks)
  • src/typegate/src/services/auth/protocols/oauth2.ts (1 hunks)
  • src/typegate/src/services/graphql_service.ts (2 hunks)
  • src/typegate/src/services/rest_service.ts (3 hunks)
  • src/typegate/src/typegate/mod.ts (1 hunks)
  • src/typegate/standalone/src/main.rs (1 hunks)
  • src/typegraph/deno/src/types.ts (0 hunks)
  • src/xtask/src/deno.rs (1 hunks)
  • tests/e2e/cli/deploy_test.ts (2 hunks)
  • tests/e2e/typegraph/validator_test.ts (1 hunks)
  • tests/planner/planner_test.ts (0 hunks)
  • tests/schema_validation/type_comparison_test.ts (1 hunks)
  • tests/utils/assert.ts (1 hunks)
  • tests/utils/s3.ts (2 hunks)
  • tests/utils/test.ts (1 hunks)
  • tools/Dockerfile (2 hunks)
  • tools/consts.ts (1 hunks)
  • tools/deps.ts (1 hunks)
  • tools/jsr/jsr-gen.ts (1 hunks)
  • tools/list-duplicates.ts (3 hunks)
  • tools/tasks/lint.ts (1 hunks)
  • tools/tree-view.ts (2 hunks)
  • tools/update.ts (2 hunks)
💤 Files with no reviewable changes (4)
  • src/typegraph/deno/src/types.ts
  • src/typegate/src/runtimes/deno/deno_messenger.ts
  • ghjk.ts
  • tests/planner/planner_test.ts
✅ Files skipped from review due to trivial changes (4)
  • src/substantial/src/backends/redis.rs
  • rust-toolchain.toml
  • import_map.json
  • tools/update.ts
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/tests.yml

93-9393: shellcheck reported issue in this script: SC2086:info:1:52: Double quote to prevent globbing and word splitting

(shellcheck)


145-145145: shellcheck reported issue in this script: SC2086:info:1:52: Double quote to prevent globbing and word splitting

(shellcheck)


175-175175: shellcheck reported issue in this script: SC2086:info:1:52: Double quote to prevent globbing and word splitting

(shellcheck)


176-176176: shellcheck reported issue in this script: SC2088:warning:1:7: Tilde does not expand in quotes. Use $HOME

(shellcheck)


176-176176: shellcheck reported issue in this script: SC2086:info:1:24: Double quote to prevent globbing and word splitting

(shellcheck)


177-177177: shellcheck reported issue in this script: SC2086:info:1:47: Double quote to prevent globbing and word splitting

(shellcheck)


288-288288: shellcheck reported issue in this script: SC2086:info:1:52: Double quote to prevent globbing and word splitting

(shellcheck)


289-289289: shellcheck reported issue in this script: SC2088:warning:1:7: Tilde does not expand in quotes. Use $HOME

(shellcheck)


289-289289: shellcheck reported issue in this script: SC2086:info:1:24: Double quote to prevent globbing and word splitting

(shellcheck)


290-290290: shellcheck reported issue in this script: SC2086:info:1:47: Double quote to prevent globbing and word splitting

(shellcheck)

.github/workflows/publish-website.yml

34-3434: shellcheck reported issue in this script: SC2086:info:1:52: Double quote to prevent globbing and word splitting

(shellcheck)

.github/workflows/release.yml

39-3939: shellcheck reported issue in this script: SC2086:info:1:52: Double quote to prevent globbing and word splitting

(shellcheck)


106-106106: shellcheck reported issue in this script: SC2086:info:1:52: Double quote to prevent globbing and word splitting

(shellcheck)


187-187187: shellcheck reported issue in this script: SC2086:info:1:52: Double quote to prevent globbing and word splitting

(shellcheck)


326-326326: shellcheck reported issue in this script: SC2086:info:1:52: Double quote to prevent globbing and word splitting

(shellcheck)

🪛 yamllint (1.35.1)
.github/workflows/tests.yml

[error] 173-173173: trailing spaces

(trailing-spaces)


[error] 284-284284: trailing spaces

(trailing-spaces)

.github/workflows/publish-website.yml

[error] 32-3232: trailing spaces

(trailing-spaces)

.github/workflows/release.yml

[error] 37-3737: trailing spaces

(trailing-spaces)


[error] 185-185185: trailing spaces

(trailing-spaces)


[error] 324-324324: trailing spaces

(trailing-spaces)

🔇 Additional comments (44)
src/mt_deno/src/lib.rs (10)

44-44: Confirm usage of PermissionFlags [approved, verify]

Switching from PermissionsOptions to args::PermissionFlags aligns with the updated permission model. Check references throughout the codebase to ensure all calls are consistent and no unforeseen breakages occur.


67-67: **Consistent update to PermissionFlags **

The parameter type change here maintains consistency with the rest of the permission handling across the file.


80-80: **Correct pass-through of updated permissions **

Passing the permissions object to args::Flags is coherent with the refactored permission system. No issues noted.


235-235: **Validate condition for empty test modules **

The newly added permit_no_files check ensures that tests with no specifiers are disallowed unless explicitly permitted. Verify that this logic matches intended behavior and doesn’t block legit scenarios where tests may be optional.


238-238: **doc_tests integration **

Acquiring doc tests from file fetcher can streamline test coverage. No immediate issues spotted; looks good for ensuring doc-based tests are included.


239-239: **Consolidating target specifiers **

Grouping specifiers for both type checking and test execution is a clear approach, centralizing these references for downstream operations.


240-240: **Iterating over doc tests **

This loop is straightforward; doc tests are processed individually. No concerns with the iteration approach.


241-241: **Inserting doc tests into memory **

Using file_fetcher.insert_memory_files(doc_test) is clever. Verify that memory insertion is tested with various doc test scenarios to avoid potential caching issues.


261-267: **Isolated Permissions for tests **

Ensuring each test obtains a fresh Permissions object prevents unintended carry-over of granted or revoked permissions between tests. This design choice appears robust.


268-268: **Running test specifiers **

Invoking test_specifiers after preparing isolated permissions is logical. Properly orchestrates final test steps.

tests/utils/assert.ts (1)

23-23: **Catching error as any **

This improves TypeScript clarity in the retry loop. It’s appropriate for a broad catch-all in test code, though stricter definitions can further enhance maintainability.

tests/e2e/typegraph/validator_test.ts (1)

30-30: **Catching error as any **

In test cases, capturing any type of error is a practical approach for robust error debugging. Current usage is fine.

tools/tasks/lint.ts (1)

50-50: **Additional deno lint command **

Executing deno lint after deno check ensures thorough static analysis. This is a beneficial improvement, reinforcing code quality checks on each run.

deno.jsonc (1)

41-41: Verify compatibility of "nodeModulesDir": "none".

Ensure that Deno 2.1.2 fully supports "none" here, as opposed to a boolean setting. Confirm there's no side effect on the project’s workflows, particularly around dependency resolution.

tools/jsr/jsr-gen.ts (1)

5-5: Removal of unused import looks good.

The existsSync import appears unused, so removing it is a welcome cleanup step.

src/typegate/standalone/src/main.rs (1)

44-49: Ensure robust error handling and environment validation.

Calling init_v8_flags is beneficial for properly initializing the V8 environment, but make sure that errors or invalid flags from get_v8_flags_from_env are properly handled or at least logged. Currently, any malformed flags could potentially cause unexpected runtime behavior or crashes.

src/typegate/src/runtimes/deno/hooks/mod.ts (1)

67-67: Good practice specifying the error type.

Marking the error as any clarifies its expected type, improving readability and helping the compiler. You may consider narrowing the error type (e.g., unknown) to avoid propagating too broad a type. That said, this is acceptable given the dynamic nature of error handling here.

src/typegate/src/runtimes/python/hooks/mod.ts (1)

57-57: Appropriate explicit typing for error.

By defining the error parameter as any, you acknowledge that Python code validation might yield unstructured exceptions. Consider using a more specific type if possible to expedite debugging, though any may be necessary when dealing with multiple error shapes.

tools/tree-view.ts (3)

18-21: Imports updated to reflect correct types.

Switching from TypeGraphDS to Typegraph and adding Injection clarifies the intended structures and improves code correctness.


25-31: Refined function signature and usage of typed injection.

Updating the treeView function’s parameter type to Typegraph and enforcing the Injection type ensures greater consistency and expressiveness in the codebase.


72-72: Maintain clarity in typed arrays.

Using Typegraph[] instead of TypeGraphDS[] to match the updated data model avoids confusion and ensures correctness when iterating over or mapping this array.

src/meta-cli/src/cli/typegate.rs (1)

71-76: Good addition of V8 flags initialization.
Invoking init_v8_flags ensures that Deno's V8 engine respects environment-specific flags. This appears consistent with the rest of the changes updating Deno functionality in this PR.

tools/deps.ts (1)

55-55: URL update looks fine.
Switching to the raw.githubusercontent.com URL for deno-udd is consistent with the project standards for direct raw content links.

tools/list-duplicates.ts (1)

15-15: Imports reorganized to include red.
It's good practice to maintain consistent import ordering for better readability. This addition clarifies usage of red for highlighting duplicate names.

tools/consts.ts (1)

8-9: Great job on version synchronization!

The Rust and Deno version constants have been updated to align with the PR objectives. This keeps the project’s versions consistent with the rest of the codebase.

src/typegate/src/services/graphql_service.ts (1)

55-59: Nice catch on consistent error naming!

Renaming the caught error from e to err clarifies the error semantics. Good practice for easier debugging and maintainability.

src/typegate/src/services/auth/mod.ts (1)

75-85: Ensure undefined handling for auth.

Switching from null to undefined requires careful handling of missing auth protocols. Check if downstream usage expects null instead.

src/xtask/src/deno.rs (2)

48-52: Initialize V8 flags early to avoid runtime conflicts.

Calling init_v8_flags with environment flags is a solid step for stable performance. Confirm that these flags don't unexpectedly impact production builds or tests.


87-99: Harden import restrictions thoughtfully.

Restricting imports by default is safer. Just ensure that required packages or domains aren’t accidentally blocked.

src/typegate/src/services/artifact_service.ts (1)

109-113: Uniform error handling looks good.

The explicit check against BaseError and fallback to UnknownError is clear and maintains consistent error reporting for various error types.

src/typegate/engine/src/lib.rs (1)

223-243: Ensure consistent test coverage for new permission flags.

The changed permission flags in the test function now mirror the production code. Verify that tests are covering scenarios where reading, writing, and importing resources are involved, to ensure all permission flags work as intended.

src/typegate/src/services/auth/protocols/oauth2.ts (1)

167-167: Confirm usage of TypeScript override modifier
Using override clarifies that this authMiddleware method is intentionally overriding a base class method, which helps detect accidental method name mismatches. This is a good practice for maintainability and clarity.

src/typegate/src/runtimes/deno/deno.ts (1)

82-82: Consider robust error handling for secret retrieval
Currently, calling secretManager.secretOrFail(secretName) will throw if the secret doesn't exist. To prevent partial initialization, consider wrapping this in a try block or logging additional context about which secret triggered the error.

src/typegate/src/typegate/mod.ts (2)

202-203: Improved clarity for caught errors
Renaming the error variable to err and typing it as any makes it clear that the error could be of various types. This also aligns with consistent naming across the codebase.


206-211: Validate error types to prevent nested error possibilities
If err.errors[0] or err.failure are missing or invalid, the code might throw a new runtime error. Consider additional checks or narrower type definitions to guard against unexpected error shapes.

src/typegate/src/runtimes/typegate.ts (1)

337-337: Correct handling of nullish coalescing
Good fix! Adding parentheses ensures that the nullish coalescing operator correctly applies to q.modelName before concatenation. The updated expression is now more precise and avoids potential undefined concatenations.

src/metagen-client-rs/src/graphql.rs (2)

372-374: Boxed response for request failures
This change adds flexibility to the response object. However, ensure the performance overhead of boxing is acceptable. In typical usage, it shouldn’t be a major concern.


421-421: Boxed GraphQLResponse
Storing GraphQLResponse in a Box can simplify the enum memory layout. Just confirm that any references to the original unboxed variant are updated accordingly.

tools/Dockerfile (2)

56-57: LGTM: Installation method change

The switch to using a shell script for GHJK installation is consistent with the changes in the workflow files.


2-3: Verify compatibility with updated versions

The version updates align with the PR objectives. However, let's verify compatibility:

✅ Verification successful

Let me gather more information to verify the compatibility.


Let me gather more specific information about Rust and Deno version requirements.


Version updates are compatible with the codebase

The version updates in the Dockerfile (Rust 1.82.0 and Deno 2.1.2) are consistent with the versions defined in tools/consts.ts and used throughout the codebase. These versions are:

  • Used in the Dockerfile for building the container
  • Referenced in the tooling configuration
  • Used in the installation scripts via tools/installs.ts

No incompatibilities or version conflicts were found in the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any compatibility issues with the new versions

# Check if Rust 1.82.0 is the minimum required version
rg -A 2 "rust-version\s*=\s*\"1\." 

# Check for any explicit Deno version requirements
rg -A 2 "deno\s+[0-9]" 

Length of output: 653


Script:

#!/bin/bash
# Check Cargo.toml files for rust-version specification and dependencies
fd Cargo.toml --exec cat {}

# Check for any Deno configuration files
fd deno.json --exec cat {}
fd deno.lock --exec cat {}

# Look for any Deno version constraints in TypeScript files
rg "Deno.version" 

Length of output: 92101


Script:

#!/bin/bash
# Check for Rust version requirements in build files and configs
rg "rust-version|RUST_VERSION" -A 2

# Check for Deno version requirements in build files and configs
rg "deno-version|DENO_VERSION" -A 2

# Check for any Deno version constraints in TypeScript files
rg "Deno\.version|deno\s+[0-9]" -A 2 "*.ts"

Length of output: 2349

Cargo.toml (2)

41-42: Minor version updates look good

The updates to clap, clap_complete, and url dependencies are minor version bumps and should maintain backward compatibility.

Also applies to: 54-54


136-137: Verify deno upgrade impact

The upgrade from Deno v1.46.3 to v2.1.2 and deno_core from 0.307.0 to 0.323.0 is significant. Let's verify the changes:

✅ Verification successful

Based on the shell script results, I can now provide the final response:

Deno upgrade impact is minimal and safe

The codebase's usage of Deno APIs is limited to stable functionality that hasn't changed significantly between v1.46.3 and v2.1.2:

  • Basic environment operations (Deno.env)
  • File system operations (Deno.mkdir, Deno.remove, etc.)
  • Network functionality (Deno.serve, Deno.Command)
  • Standard utilities (Deno.inspect)

The deno_core usage is primarily for op definitions which remain compatible across the versions, with no usage of deprecated or removed APIs.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for potential breaking changes

# Check for Deno API usage that might be affected
rg -A 3 "Deno\." src/
rg -A 3 "deno_core::" src/

# Check for any custom deno types
ast-grep --pattern 'interface $name extends Deno.$rest { $$$  }'

Length of output: 31996

.github/workflows/tests.yml (1)

32-32: LGTM: Added debug logging

Adding GHJK_LOG="debug" will help with troubleshooting CI issues.

.ghjk/lock.json (1)

734-744: Version updates look good!

The updates align with the PR objectives:

  • Added Deno v2.1.2 configuration
  • Updated Rust to v1.82.0

Also applies to: 2259-2259

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (12)
tests/metagen/typegraphs/sample/py/client.py (5)

812-812: Consider adding docstrings to clarify the purpose of this method.

The newly added Post() method appears straightforward, but adding a brief docstring explaining its role in defining the schema would enhance maintainability and readability for future contributors.


967-975: Clarity and consistency: add docstrings to reflect the User structure.

The User TypedDict has fields referencing specialized types (like UserIdStringUuid). Consider adding docstrings or short comments about each field's usage or constraints, especially as the project evolves.


1044-1052: Initialization suggestions.

The mapping {"UserIdStringUuid": "String!", "PostSlugString": "String!", ...} is a minimal type-to-GraphQL mapping. If more types are added in the future, consider organizing them in a dedicated configuration or constants file to avoid repetition.


1063-1065: Check for symmetrical patterns in get_posts.

This block mirrors get_user. If you foresee more custom logic in the future (like complex filters), consider factoring out the common code into a helper function.


1089-1095: Use consistent naming across no-args calls.

composite_no_args is fine. Should you have multiple similar no-argument functions, keep consistent naming to reduce confusion in usage.

.github/workflows/tests.yml (3)

32-32: Check potential verbosity from GHJK_LOG debug setting.

While setting GHJK_LOG to debug aids troubleshooting, consider toggling it off or lowering verbosity outside of debug scenarios to prevent log clutter.


173-173: Remove trailing whitespace.

A minor YAML lint issue: line 173 has trailing spaces. It’s best practice to remove them for a cleaner version history and to satisfy lint checks.

-      
+
🧰 Tools
🪛 yamllint (1.35.1)

[error] 173-173: trailing spaces

(trailing-spaces)


284-284: Remove trailing whitespace.

Line 284 contains trailing spaces. This can break certain lint or YAML validation rules.

-      
+
🧰 Tools
🪛 yamllint (1.35.1)

[error] 284-284: trailing spaces

(trailing-spaces)

.github/workflows/release.yml (4)

37-37: Remove trailing whitespace.

Line 37 has trailing spaces. Removing unnecessary whitespace promotes cleaner diffs.

-      
+
🧰 Tools
🪛 yamllint (1.35.1)

[error] 37-37: trailing spaces

(trailing-spaces)


106-110: Consistent approach to ghjk setup.

Repetitive block for installing ghjk. If you have many workflows, consider centralizing this logic in a reusable Action or script to reduce duplication.


185-185: Remove trailing whitespace.

Line 185 has trailing spaces. Clear them to maintain consistent formatting.

-      
+
🧰 Tools
🪛 yamllint (1.35.1)

[error] 185-185: trailing spaces

(trailing-spaces)


324-324: Remove trailing whitespace.

Line 324 has trailing spaces. Consider cleaning these up to stay consistent with linting rules.

-      
+
🧰 Tools
🪛 yamllint (1.35.1)

[error] 324-324: trailing spaces

(trailing-spaces)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7d2be27 and cd7db98.

⛔ Files ignored due to path filters (2)
  • Cargo.lock is excluded by !**/*.lock
  • deno.lock is excluded by !**/*.lock
📒 Files selected for processing (8)
  • .github/workflows/release.yml (4 hunks)
  • .github/workflows/tests.yml (5 hunks)
  • Cargo.toml (4 hunks)
  • src/metagen-client-rs/Cargo.toml (1 hunks)
  • tests/metagen/typegraphs/sample/py/client.py (3 hunks)
  • tests/runtimes/deno/ts/math.ts (1 hunks)
  • tools/Dockerfile.dockerignore (1 hunks)
  • tools/list-duplicates.ts (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • tools/list-duplicates.ts
  • Cargo.toml
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/tests.yml

176-176: shellcheck reported issue in this script: SC2088:warning:1:7: Tilde does not expand in quotes. Use $HOME

(shellcheck)


176-176: shellcheck reported issue in this script: SC2086:info:1:24: Double quote to prevent globbing and word splitting

(shellcheck)


177-177: shellcheck reported issue in this script: SC2086:info:1:47: Double quote to prevent globbing and word splitting

(shellcheck)


289-289: shellcheck reported issue in this script: SC2088:warning:1:7: Tilde does not expand in quotes. Use $HOME

(shellcheck)


289-289: shellcheck reported issue in this script: SC2086:info:1:24: Double quote to prevent globbing and word splitting

(shellcheck)


290-290: shellcheck reported issue in this script: SC2086:info:1:47: Double quote to prevent globbing and word splitting

(shellcheck)

🪛 yamllint (1.35.1)
.github/workflows/tests.yml

[error] 173-173: trailing spaces

(trailing-spaces)


[error] 284-284: trailing spaces

(trailing-spaces)

.github/workflows/release.yml

[error] 37-37: trailing spaces

(trailing-spaces)


[error] 185-185: trailing spaces

(trailing-spaces)


[error] 324-324: trailing spaces

(trailing-spaces)

🔇 Additional comments (25)
src/metagen-client-rs/Cargo.toml (2)

6-10: LGTM! Well-documented optimization strategy

The comments clearly explain the Docker cache optimization strategy, and the library configuration follows Rust conventions.


Line range hint 13-24: Verify dependency compatibility with Rust 1.82.0

The dependencies appear to be using recent versions. Let's verify their compatibility with Rust 1.82.0.

✅ Verification successful

Let me gather more information to verify the compatibility.


Dependencies are compatible with Rust 1.82.0

Based on the verification results:

  • All dependencies are using stable versions that are compatible with Rust 1.82.0
  • reqwest 0.12.9, serde 1.0.210, and other dependencies are recent releases from their respective stable branches
  • No MSRV (Minimum Supported Rust Version) constraints were found that would conflict with Rust 1.82.0
  • Only one deprecated package was found in the dependency tree (0.9.34+deprecated), but it's a transitive dependency and doesn't affect compatibility

The dependency versions look good and should work well with Rust 1.82.0. No compatibility issues were found.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if dependencies are compatible with Rust 1.82.0
# Expected: No compatibility issues reported

# Check for any known compatibility issues with these dependencies
gh api graphql -f query='
{
  securityVulnerabilities(first: 10, ecosystem: RUST, package: "serde") {
    nodes {
      advisory {
        summary
        severity
        affectedVersions
      }
    }
  }
}'

# List any deprecated features or compatibility warnings
rg -A 2 'deprecated|WARN' Cargo.lock || true

Length of output: 802


Script:

#!/bin/bash
# Check Cargo.toml for MSRV (Minimum Supported Rust Version) specifications
rg -A 2 'rust-version|MSRV' Cargo.toml

# Check dependencies' versions in Cargo.lock
rg -A 2 'name = "(serde|reqwest|mime_guess|futures|tokio-util|derive_more|lazy_static)"' Cargo.lock

# Look for any compatibility warnings in the repository
rg -i 'requires?.*(rust|version).*[0-9]+\.[0-9]+' . --type rust

Length of output: 1267

tests/runtimes/deno/ts/math.ts (2)

4-5: Consider clarifying license usage for external code reference.
You reference an external code link in the file comments. Ensure compatibility with MPL-2.0 and add relevant attribution if required by the external project's license.


7-7: Verify pinned version of Big.js.
You're specifying Big.js at version 6.2.2, which may be outdated. Confirm that this version suits your needs and is free of known vulnerabilities or consider upgrading to the latest stable version for security patches and new features.

✅ Verification successful

Big.js version 6.2.2 is the latest available version

The project is already using the latest stable version (6.2.2) of Big.js from npm registry, so no upgrade is necessary at this time.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for the latest version of Big.js on npm:
curl -s "https://registry.npmjs.org/big.js" | jq '."dist-tags"'

Length of output: 87

tests/metagen/typegraphs/sample/py/client.py (14)

940-940: Good practice: align naming conventions with usage context.

Declaring UserIdStringUuid = str is a quick way to alias a str type. Ensure that this naming convention is consistent across the entire codebase and that each usage of UserIdStringUuid remains self-explanatory.


945-953: TypedDict definitions look consistent.

These new lines correctly define the Post TypedDict with appropriate fields and type aliases. Although optional, consider adding docstrings or comments indicating the meaning of each field for improved clarity.


955-961: Keep input schema well-documented.

RootCompositeArgsFnInput is defined accurately. To avoid confusion, consider adding a comment or docstring clarifying its usage in method calls within the codebase (e.g., in composite_args).


999-1008: Avoid potential confusion with naming.

PostSelections is a clear, descriptive name for a selection set. Overall, the typed definitions are consistent. If at any point the structure grows more complex, consider grouping these definitions in a dedicated module.


1010-1019: Good maintainability approach in UserSelections.

This definition cleanly separates user field selections from PostSelections. The code is well-structured and consistent with the rest of the typed selection blocks.


1021-1030: Successful union approach for RootCompositeUnionFnOutputSelections.

The typed dict structure is well-defined, capturing union members that can refer to either post or user sub-selections. This helps keep complex union logic cohesive.


1031-1039: Check for future expansion.

RootMixedUnionFnOutputSelections merges multiple union possibilities. This is fine for current usage. If more variants appear in the future, ensure naming remains intuitive (e.g., rename if more complex types are introduced).


1055-1057: Validation checks for getUser selection.

The usage of selection_to_nodes is correct, but remember to add unit tests to confirm that invalid or missing fields in select raise proper exceptions.


1071-1073: scalarNoArgs usage is correct.

No arguments are required, so this direct approach is fine. Be sure to cover it in tests verifying the query’s return value for typical and edge-cases.


1077-1085: Arg-based mutation logic.

scalar_args can be expanded to use more specific typed dictionaries if needed. Currently, it’s properly referencing Post for argument structure. Consider verifying the shape of args in a test suite.


1097-1109: Argument handling in composite_args is well structured.

The typed input RootCompositeArgsFnInput is used effectively. This keeps the system strongly typed. A thorough integration test would ensure that the typed dictionary and selection objects remain in sync.


1111-1119: Union logic checks.

scalar_union merges different types under a single query node. Remain vigilant about shape mismatches in arguments or selections. If necessary, add logic for safer runtime type checks.


1121-1133: Solid union pattern in composite_union.

This method properly handles union variants. If you plan to expand variants in the future, ensure compatibility with the selection transformations in NodeDescs.


1135-1147: No issues in mixed_union.

The code is consistent with the structure used in other union-based queries. Just ensure the project’s unit or integration tests validate the multiple union scenarios.

.github/workflows/tests.yml (4)

91-97: Shell steps to install ghjk appear correct.

Replacing the setup-ghjk action with direct commands is valid. Just ensure secure handling of environment variables and that the installed version is pinned or verified for reproducible builds.


140-149: Repeat patterns in shell commands.

These lines reuse the method of installing ghjk. Watch for any drift in version references. Confirm that all references to $GHJK_VERSION are consistent.


176-179: Shell path expansions need double quoting and $HOME usage.

ShellCheck warns about tilde expansions within quotes. Use $HOME in place of ~ and wrap variables in quotes to avoid word splitting or glob expansion.

-      - run: echo "~/.local/bin" >> $GITHUB_PATH
-      - run: echo "BASH_ENV=~/.local/share/ghjk/env.sh" >> $GITHUB_ENV
+      - run: echo "$HOME/.local/bin" >> "$GITHUB_PATH"
+      - run: echo "BASH_ENV=$HOME/.local/share/ghjk/env.sh" >> "$GITHUB_ENV"
🧰 Tools
🪛 actionlint (1.7.4)

176-176: shellcheck reported issue in this script: SC2088:warning:1:7: Tilde does not expand in quotes. Use $HOME

(shellcheck)


176-176: shellcheck reported issue in this script: SC2086:info:1:24: Double quote to prevent globbing and word splitting

(shellcheck)


177-177: shellcheck reported issue in this script: SC2086:info:1:47: Double quote to prevent globbing and word splitting

(shellcheck)


289-292: Duplicate shell script improvements.

As above, use $HOME/.local/bin and quote variables to avoid expansions. This ensures consistency and satisfies ShellCheck and actionlint warnings.

-      - run: echo "~/.local/bin" >> $GITHUB_PATH
-      - run: echo "BASH_ENV=~/.local/share/ghjk/env.sh" >> $GITHUB_ENV
+      - run: echo "$HOME/.local/bin" >> "$GITHUB_PATH"
+      - run: echo "BASH_ENV=$HOME/.local/share/ghjk/env.sh" >> "$GITHUB_ENV"
🧰 Tools
🪛 actionlint (1.7.4)

289-289: shellcheck reported issue in this script: SC2088:warning:1:7: Tilde does not expand in quotes. Use $HOME

(shellcheck)


289-289: shellcheck reported issue in this script: SC2086:info:1:24: Double quote to prevent globbing and word splitting

(shellcheck)


290-290: shellcheck reported issue in this script: SC2086:info:1:47: Double quote to prevent globbing and word splitting

(shellcheck)

.github/workflows/release.yml (3)

39-43: Installation steps match the rest of the workflows.

Mirrors the pattern of manually installing ghjk in place of the old action. Looks consistent. Confirm that the environment variables are handled in the same way as other workflows.


186-191: Proper shell steps, but watch quoting.

Like previous comparisons, ensure $HOME is used instead of ~ to avoid ShellCheck SC2088. Also confirm that you double quote all expansions.


326-330: Installation consistency for ghjk.

The pattern continues: cURL, add local bin to path, set BASH_ENV, and run ghjk envs cook. This is acceptable, but consider a centralized snippet if used across multiple workflows.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🔭 Outside diff range comments (1)
tools/list-duplicates.ts (1)

Line range hint 81-104: Remove commented-out code.

Instead of keeping the old file-based input code commented out, it should be removed. If needed, the code can be retrieved from version control history.

♻️ Duplicate comments (1)
.github/workflows/tests.yml (1)

289-291: ⚠️ Potential issue

Fix shell script security issues.

Same issues as found in the test-website job.

🧰 Tools
🪛 actionlint (1.7.4)

289-289: shellcheck reported issue in this script: SC2088:warning:1:7: Tilde does not expand in quotes. Use $HOME

(shellcheck)


289-289: shellcheck reported issue in this script: SC2086:info:1:24: Double quote to prevent globbing and word splitting

(shellcheck)


290-290: shellcheck reported issue in this script: SC2086:info:1:47: Double quote to prevent globbing and word splitting

(shellcheck)

🧹 Nitpick comments (3)
src/typegate/src/runtimes/s3.ts (1)

96-96: Prefer explicit S3 object type over any.

While using any might quickly silence type complaints, it undermines the benefits of static typing. If the AWS SDK provides a specific shape for res.Contents, please consider using that shape (for example, Object or ObjectList from aws-sdk/client-s3) to ensure you have correct types across the codebase.

-keys: (res.Contents ?? []).map((c: any) => ({
+keys: (res.Contents ?? []).map((c: Object) => ({
    key: c.Key,
    size: c.Size,
}))
tools/list-duplicates.ts (2)

22-23: Consider encapsulating counters in a result object.

Using global mutable state can make the code harder to test and debug. Consider refactoring to return these metrics as part of a result object from listDuplicates.

-let whereTypeCount = 0;
-let optionalTypeCount = 0;
+interface DuplicateAnalysis {
+  whereTypeCount: number;
+  optionalTypeCount: number;
+  duplicates: Map<string, readonly [number, TypeNode][]>;
+}
+
-export function listDuplicates(tg: TypeGraphDS, rootIdx = 0) {
+export function listDuplicates(tg: TypeGraphDS, rootIdx = 0): DuplicateAnalysis {
+  let whereTypeCount = 0;
+  let optionalTypeCount = 0;
   // ... existing code ...
+  return {
+    whereTypeCount,
+    optionalTypeCount,
+    duplicates: bins
+  };
}

50-51: Consider extracting duplicate injection handling logic.

The injection handling code is duplicated. Consider extracting it into a helper function for better maintainability.

+function formatInjection(type: TypeNode): string {
+  // deno-lint-ignore no-explicit-any
+  return "injection" in type ? ` (injection ${(type.injection as any).source})` : "";
+}

// Then use it in both places:
-const injection = "injection" in type
-  // deno-lint-ignore no-explicit-any
-  ? ` (injection ${(type.injection as any).source})`
-  : "";
+const injection = formatInjection(type);

Also applies to: 64-65

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cd7db98 and b924886.

📒 Files selected for processing (5)
  • .github/workflows/release.yml (5 hunks)
  • .github/workflows/tests.yml (5 hunks)
  • src/meta-cli/src/macros.rs (1 hunks)
  • src/typegate/src/runtimes/s3.ts (1 hunks)
  • tools/list-duplicates.ts (4 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/tests.yml

176-176: shellcheck reported issue in this script: SC2088:warning:1:7: Tilde does not expand in quotes. Use $HOME

(shellcheck)


176-176: shellcheck reported issue in this script: SC2086:info:1:24: Double quote to prevent globbing and word splitting

(shellcheck)


177-177: shellcheck reported issue in this script: SC2086:info:1:47: Double quote to prevent globbing and word splitting

(shellcheck)


289-289: shellcheck reported issue in this script: SC2088:warning:1:7: Tilde does not expand in quotes. Use $HOME

(shellcheck)


289-289: shellcheck reported issue in this script: SC2086:info:1:24: Double quote to prevent globbing and word splitting

(shellcheck)


290-290: shellcheck reported issue in this script: SC2086:info:1:47: Double quote to prevent globbing and word splitting

(shellcheck)

🪛 yamllint (1.35.1)
.github/workflows/tests.yml

[error] 173-173: trailing spaces

(trailing-spaces)


[error] 284-284: trailing spaces

(trailing-spaces)

.github/workflows/release.yml

[error] 37-37: trailing spaces

(trailing-spaces)


[error] 185-185: trailing spaces

(trailing-spaces)


[error] 324-324: trailing spaces

(trailing-spaces)

⏰ Context from checks skipped due to timeout of 90000ms (7)
  • GitHub Check: bulid-docker (linux/arm64, custom-arm)
  • GitHub Check: test-website
  • GitHub Check: test-full
  • GitHub Check: lint-compat (macos-14, aarch64-apple-darwin, false)
  • GitHub Check: bulid-docker (linux/amd64, custom-ubuntu-large)
  • GitHub Check: lint-compat (macos-13, x86_64-apple-darwin, false)
  • GitHub Check: pre-commit
🔇 Additional comments (6)
src/meta-cli/src/macros.rs (1)

13-17: Confirm the 'static trait object usage for concurrency and error chaining.

This update properly boxes the error as a trait object that is both Send and Sync, which is beneficial when errors need to be passed across threads. However, confirm that forcing 'static is necessary and doesn’t accidentally cut off useful error context or references. If this accommodation is intended for concurrency or error propagation, then the change is sound. Otherwise, consider whether 'static is strictly required.

tools/list-duplicates.ts (2)

8-8: LGTM! Usage documentation updated correctly.

The usage comment and import organization accurately reflect the changes in input handling method.

Also applies to: 15-15


30-42: LGTM! Type processing logic is sound.

The implementation correctly:

  • Handles type structure hashing
  • Tracks duplicate names consistently
  • Counts specific type patterns
.github/workflows/tests.yml (2)

176-178: ⚠️ Potential issue

Fix shell script security issues.

The commands need proper quoting and should use $HOME instead of tilde.

Apply this fix:

-      - run: echo "~/.local/bin" >> $GITHUB_PATH
-      - run: echo "BASH_ENV=~/.local/share/ghjk/env.sh" >> $GITHUB_ENV
+      - run: echo "$HOME/.local/bin" >> "$GITHUB_PATH"
+      - run: echo "BASH_ENV=$HOME/.local/share/ghjk/env.sh" >> "$GITHUB_ENV"

Likely invalid or redundant comment.

🧰 Tools
🪛 actionlint (1.7.4)

176-176: shellcheck reported issue in this script: SC2088:warning:1:7: Tilde does not expand in quotes. Use $HOME

(shellcheck)


176-176: shellcheck reported issue in this script: SC2086:info:1:24: Double quote to prevent globbing and word splitting

(shellcheck)


177-177: shellcheck reported issue in this script: SC2086:info:1:47: Double quote to prevent globbing and word splitting

(shellcheck)


Line range hint 61-61: Verify version tagging strategy change.

The prerelease condition now checks for 'rc' instead of previous version tags.

Let's check the existing tags to ensure this change aligns with the project's versioning strategy:

✅ Verification successful

I don't see any prerelease condition checking for 'rc' or 'alpha' in the provided workflow file. Let me check other workflow files that might contain version tagging logic.


I found the relevant prerelease condition in the release.yml workflow file. The condition is used in the check-bump job to determine if a release should be marked as prerelease:

prerelease: ${{ contains(steps.latest-tag.outputs.tag, 'dev') || contains(steps.latest-tag.outputs.tag, 'rc') }}

Let me check if there are any other files that might be affected by this version tagging strategy.


Let me try a different search approach without file type restrictions.


Let me check the actual content of these files to see if they're related to version tagging.


Based on the analysis of the workflow files and codebase, I can now provide a final response:

Version tagging strategy change is aligned with the project's conventions

The change from 'alpha' to 'rc' in the prerelease condition is consistent with the project's versioning strategy. The release workflow (.github/workflows/release.yml) already handles both 'rc' and 'dev' tags as prereleases:

prerelease: ${{ contains(steps.latest-tag.outputs.tag, 'dev') || contains(steps.latest-tag.outputs.tag, 'rc') }}

This change appears to be an intentional shift from alpha to release candidate (rc) versioning, which is a common progression in semantic versioning.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check existing version tags
git tag -l "v*" | grep -E 'alpha|rc|dev'

Length of output: 42


Script:

#!/bin/bash
# Let's check the workflow file to understand the version tagging logic
cat .github/workflows/tests.yml

Length of output: 12203


Script:

#!/bin/bash
# Let's check all workflow files for version tagging logic
fd -e yml -e yaml . .github/workflows/ --exec cat {}

Length of output: 31379


Script:

#!/bin/bash
# Search for files containing version tag related patterns
rg -l "alpha|rc|dev" --type yaml --type rust --type typescript

Length of output: 99


Script:

#!/bin/bash
# Search for files containing version tag related patterns
rg -l "alpha|rc|dev" --type yaml

Length of output: 670


Script:

#!/bin/bash
# Check content of relevant workflow and configuration files
echo "=== .gitlab-ci.yml ==="
cat .gitlab-ci.yml
echo -e "\n=== whiz.yaml ==="
cat whiz.yaml

Length of output: 1862

.github/workflows/release.yml (1)

37-43: LGTM! Consistent and secure installation commands.

All instances of the ghjk installation commands are properly implemented with correct quoting and environment variable usage.

Also applies to: 106-110, 185-191, 324-330

🧰 Tools
🪛 yamllint (1.35.1)

[error] 37-37: trailing spaces

(trailing-spaces)

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (21)
tests/metagen/typegraphs/sample/rs_upload/Cargo.toml (1)

7-7: Changes are consistent across test packages.

The dependency change matches the one in sample_client/Cargo.toml, maintaining consistency. However, please:

  1. Document the rationale for moving away from workspace dependencies
  2. Consider adding a comment in both Cargo.toml files explaining this setup
tests/tools/schema_test.ts (2)

4-6: Consider using import_map.json for version management.

While directly specifying versions in URLs works, consider moving these version specifications to an import map for better maintainability.


32-39: Consider enhancing error messages with validation details.

When validation fails, include the specific validation errors from Ajv to help developers quickly identify and fix issues.

-    assert(result, `validation failed for '${file}'`);
+    if (!result) {
+      assert(false, `Validation failed for '${file}': ${JSON.stringify(validate.errors, null, 2)}`);
+    }
tests/metagen/typegraphs/sample.ts (1)

47-56: Consider adding JSDoc comments for the nested structure.

The nestedComposite structure is well-designed, but documentation would help explain its purpose and usage.

+/**
+ * Represents a nested structure with scalar, composite, and list fields.
+ * @property {number} scalar - An integer value
+ * @property {object} composite - A nested structure containing value and further nesting
+ * @property {Array<{value: number}>} list - A list of objects with integer values
+ */
 const nestedComposite = t.struct({
   scalar: t.integer(),
   composite: t.struct({
tests/metagen/typegraphs/sample/rs/client.rs (2)

268-287: Consider adding documentation for the partial structures.

While the implementation is correct, adding documentation comments would help explain:

  • The purpose of each partial structure
  • The relationship between these structures
  • Usage examples

Apply this diff to add documentation:

+/// Represents the nested structure within a composite structure
 #[derive(Debug, serde::Serialize, serde::Deserialize)]
 pub struct RootNestedCompositeFnOutputCompositeStructNestedStructPartial {
     pub inner: Option<RootScalarUnionFnOutputT1Integer>,
 }

+/// Represents a composite structure that can contain nested structures
 #[derive(Debug, serde::Serialize, serde::Deserialize)]
 pub struct RootNestedCompositeFnOutputCompositeStructPartial {
     pub value: Option<RootScalarUnionFnOutputT1Integer>,
     pub nested: Option<RootNestedCompositeFnOutputCompositeStructNestedStructPartial>,
 }

315-337: Consider adding derive macros for common traits.

The selection structures could benefit from implementing additional traits for better ergonomics.

Apply this diff to add common traits:

-#[derive(Default, Debug)]
+#[derive(Default, Debug, Clone, PartialEq)]
 pub struct RootNestedCompositeFnOutputCompositeStructNestedStructSelections<ATy = NoAlias> {
     pub inner: ScalarSelect<ATy>,
 }
src/metagen/src/client_py/static/client.py (1)

20-25: Improve boolean expression readability

The boolean expression can be simplified using the built-in bool() function.

-    select_all = True if sub_flags is not None and sub_flags.select_all else False
+    select_all = bool(sub_flags is not None and sub_flags.select_all)
🧰 Tools
🪛 Ruff (0.8.2)

25-25: Use bool(...) instead of True if ... else False

Replace with `bool(...)

(SIM210)

tests/metagen/typegraphs/sample/py_upload/client.py (1)

23-28: Improve boolean expression readability

Similar to the previous file, the boolean expression can be simplified.

-    select_all = True if sub_flags is not None and sub_flags.select_all else False
+    select_all = bool(sub_flags is not None and sub_flags.select_all)
🧰 Tools
🪛 Ruff (0.8.2)

28-28: Use bool(...) instead of True if ... else False

Replace with `bool(...)

(SIM210)

tools/list-duplicates.ts (5)

21-22: Consider using a more structured approach for counters.

The global variables could lead to issues with state management, especially when processing multiple type graphs.

Consider encapsulating the counters in a class or returning them as part of the function result:

-let whereTypeCount = 0;
-let optionalTypeCount = 0;
+interface TypeCounts {
+  whereTypeCount: number;
+  optionalTypeCount: number;
+}
+
+class TypeCounter {
+  private whereTypeCount = 0;
+  private optionalTypeCount = 0;
+
+  increment(type: 'where' | 'optional'): void {
+    if (type === 'where') this.whereTypeCount++;
+    else this.optionalTypeCount++;
+  }
+
+  getCounts(): TypeCounts {
+    return {
+      whereTypeCount: this.whereTypeCount,
+      optionalTypeCount: this.optionalTypeCount
+    };
+  }
+}

23-41: Improve selection flag handling.

The selection flag handling could be more robust with better type checking and error messages.

Consider these improvements:

-    sub_flags = selection.get("_")
-    if sub_flags is not None and not isinstance(sub_flags, SelectionFlags):
+    const sub_flags = selection.get("_");
+    if (sub_flags !== undefined && !(sub_flags instanceof SelectionFlags)) {
         throw new Error(
-            f"selection field '_' should be of type SelectionFlags but found {type(sub_flags)}"
+            `selection field '_' should be of type SelectionFlags but found ${typeof sub_flags}`
         );
     }

28-28: Replace conditional with direct boolean conversion.

The boolean conversion can be simplified.

-    select_all = True if sub_flags is not None and sub_flags.select_all else False
+    select_all = Boolean(sub_flags?.select_all)

36-41: Consider using a more descriptive pattern for type matching.

The pattern matching for _where_ could be more specific to avoid false positives.

-    if (title.match(/_where_/)) {
+    if (title.match(/^.*_where_.*$/)) {
       whereTypeCount++;
     }
-    if (type.type == "optional") {
+    if (type.type === "optional") {
       optionalTypeCount++;
     }

117-117: Enhance counter logging with more context.

The counter logging could be more informative.

-console.log({ whereTypeCount, optionalTypeCount });
+console.log({
+  stats: {
+    whereTypeCount,
+    optionalTypeCount,
+  },
+  message: 'Type counting complete'
+});
tools/schema/metatype.json (4)

1-6: Add schema metadata for better discoverability.

The schema could benefit from additional metadata fields.

 {
   "$schema": "http://json-schema.org/draft-07/schema#",
   "$id": "https://json.schemastore.org/metatype.json",
   "title": "Metatype configuration file schema",
+  "description": "Configuration schema for Metatype framework",
+  "type": "object",
+  "$comment": "Schema version: 1.0.0",
   "additionalProperties": false,
-  "type": "object",

204-208: Strengthen URL validation pattern.

The URL format validation could be more specific.

     "url": {
       "type": "string",
       "format": "uri",
+      "pattern": "^https?://[\\w\\-\\.]+(:\\d+)?(/.*)?$",
       "description": "The base URL of the typegate server, example: <http://localhost:7890>."
     },

221-227: Add pattern validation for environment variables.

Environment variables should follow a standard naming convention.

     "env": {
       "type": "object",
       "description": "Environment variables for the typegate.",
       "additionalProperties": {
-        "type": "string"
+        "type": "string",
+        "pattern": "^[A-Z][A-Z0-9_]*$"
       }
     },

268-271: Add path validation for migrations directory.

The migrations path should follow standard path conventions.

     "migrations_path": {
       "type": "string",
+      "pattern": "^[\\w\\-\\.\\/_]+$",
       "description": "The directory for storing Prisma migration files."
     }
tests/metagen/metagen_test.ts (3)

37-37: Consider using a custom logger for S3 operations.

Using console directly for S3 client logging could mix operational logs with test output.

-    logger: console,
+    logger: {
+      debug: (...args) => console.debug('[S3]', ...args),
+      info: (...args) => console.info('[S3]', ...args),
+      warn: (...args) => console.warn('[S3]', ...args),
+      error: (...args) => console.error('[S3]', ...args)
+    },

42-45: Improve error handling in bucket preparation.

The error handling for bucket creation could be more specific.

-  } catch (err: any) {
-    if (err?.Code != "BucketAlreadyOwnedByYou") {
-      throw err;
-    }
+  } catch (err) {
+    if (err instanceof Error) {
+      if ('Code' in err && err.Code === 'BucketAlreadyOwnedByYou') {
+        console.info('Bucket already exists and is owned by you');
+        return;
+      }
+      throw new Error(`Failed to create bucket: ${err.message}`);
+    }
+    throw err;

598-615: Add validation for nested composite schema.

The schema definition for nested composites could benefit from additional validation.

     const expectedSchemaC = zod.object({
       scalarOnly: zod.object({ scalar: zod.number() }),
       withStruct: zod.object({
         scalar: zod.number(),
-        composite: zod.object({ value: zod.number() }),
+        composite: zod.object({
+          value: zod.number().min(0),
+          description: zod.string().optional()
+        }),
       }),
       withStructNested: zod.object({
         scalar: zod.number(),
         composite: zod.object({
-          value: zod.number(),
+          value: zod.number().min(0),
           nested: zod.object({ inner: zod.number() }),
         }),
       }),
       withList: zod.object({
         scalar: zod.number(),
-        list: zod.array(zod.object({ value: zod.number() })),
+        list: zod.array(zod.object({
+          value: zod.number().min(0),
+          description: zod.string().optional()
+        })).min(1),
       }),
     });
tests/metagen/typegraphs/sample/py/client.py (1)

23-28: Simplify boolean expression and improve variable naming.

The variable rename from flags to sub_flags improves clarity. However, the boolean expression can be simplified.

Apply this diff to simplify the code:

-    select_all = True if sub_flags is not None and sub_flags.select_all else False
+    select_all = bool(sub_flags is not None and sub_flags.select_all)
🧰 Tools
🪛 Ruff (0.8.2)

28-28: Use bool(...) instead of True if ... else False

Replace with `bool(...)

(SIM210)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b924886 and 58673a1.

⛔ Files ignored due to path filters (6)
  • .ghjk/deno.lock is excluded by !**/*.lock
  • Cargo.lock is excluded by !**/*.lock
  • deno.lock is excluded by !**/*.lock
  • tests/metagen/typegraphs/sample/rs/Cargo.lock is excluded by !**/*.lock
  • tests/metagen/typegraphs/sample/rs_upload/Cargo.lock is excluded by !**/*.lock
  • tests/runtimes/wasm_reflected/rust/Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (40)
  • .ghjk/lock.json (14 hunks)
  • examples/templates/deno/api/example.ts (1 hunks)
  • examples/typegraphs/metagen/rs/fdk.rs (1 hunks)
  • ghjk.ts (1 hunks)
  • import_map.json (1 hunks)
  • src/metagen-client-rs/src/selection.rs (1 hunks)
  • src/metagen/src/client_py/static/client.py (3 hunks)
  • src/metagen/src/client_ts/static/mod.ts (1 hunks)
  • src/substantial/Cargo.toml (1 hunks)
  • src/substantial/src/protocol/events.rs (2 hunks)
  • src/substantial/src/protocol/metadata.rs (2 hunks)
  • src/typegate/src/runtimes/deno/deno_messenger.ts (1 hunks)
  • src/typegate/src/runtimes/deno/hooks/mod.ts (4 hunks)
  • src/typegate/src/runtimes/substantial/workflow_worker_manager.ts (1 hunks)
  • src/typegate/src/typegate/hooks.ts (2 hunks)
  • src/typegate/src/typegate/mod.ts (1 hunks)
  • tests/e2e/published/published_test.ts (4 hunks)
  • tests/injection/random_injection_test.ts (1 hunks)
  • tests/metagen/metagen_test.ts (3 hunks)
  • tests/metagen/typegraphs/identities/rs/fdk.rs (1 hunks)
  • tests/metagen/typegraphs/sample.ts (1 hunks)
  • tests/metagen/typegraphs/sample/py/client.py (6 hunks)
  • tests/metagen/typegraphs/sample/py/main.py (1 hunks)
  • tests/metagen/typegraphs/sample/py_upload/client.py (3 hunks)
  • tests/metagen/typegraphs/sample/rs/Cargo.toml (1 hunks)
  • tests/metagen/typegraphs/sample/rs/client.rs (4 hunks)
  • tests/metagen/typegraphs/sample/rs/main.rs (2 hunks)
  • tests/metagen/typegraphs/sample/rs_upload/Cargo.toml (1 hunks)
  • tests/metagen/typegraphs/sample/ts/client.ts (5 hunks)
  • tests/metagen/typegraphs/sample/ts/main.ts (2 hunks)
  • tests/metagen/typegraphs/sample/ts_upload/client.ts (1 hunks)
  • tests/runtimes/deno/deno_sync_test.ts (1 hunks)
  • tests/runtimes/wasm_wire/rust/fdk.rs (1 hunks)
  • tests/tools/schema_test.ts (1 hunks)
  • tests/utils/process.ts (1 hunks)
  • tools/installs.ts (1 hunks)
  • tools/list-duplicates.ts (4 hunks)
  • tools/schema/metatype.json (1 hunks)
  • tools/tasks/gen.ts (1 hunks)
  • tools/tasks/lint.ts (1 hunks)
✅ Files skipped from review due to trivial changes (5)
  • src/substantial/Cargo.toml
  • examples/templates/deno/api/example.ts
  • tests/metagen/typegraphs/identities/rs/fdk.rs
  • src/substantial/src/protocol/metadata.rs
  • src/substantial/src/protocol/events.rs
🚧 Files skipped from review as they are similar to previous changes (5)
  • src/typegate/src/runtimes/deno/deno_messenger.ts
  • src/typegate/src/typegate/mod.ts
  • ghjk.ts
  • tools/tasks/lint.ts
  • src/typegate/src/runtimes/deno/hooks/mod.ts
🧰 Additional context used
🪛 Ruff (0.8.2)
tests/metagen/typegraphs/sample/py_upload/client.py

28-28: Use bool(...) instead of True if ... else False

Replace with `bool(...)

(SIM210)

src/metagen/src/client_py/static/client.py

25-25: Use bool(...) instead of True if ... else False

Replace with `bool(...)

(SIM210)

tests/metagen/typegraphs/sample/py/client.py

28-28: Use bool(...) instead of True if ... else False

Replace with `bool(...)

(SIM210)

🔇 Additional comments (38)
tools/tasks/gen.ts (2)

22-22: Verify protobuf-codegen version compatibility

The version update to 3.7.1 aligns with the changes in other files. Let's verify this is the latest compatible version.

✅ Verification successful

✓ Protobuf version 3.7.1 is consistently configured

The version update to 3.7.1 is correctly aligned between the Rust crate dependency and the TypeScript tooling configuration, with no conflicting versions found in the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check latest version of protobuf-codegen
cargo search protobuf-codegen --limit 1

# Check if this version is used consistently
rg -l "protobuf.*3\.7\.1" 

Length of output: 291


Script:

#!/bin/bash
# Check the Cargo.toml content
cat src/substantial/Cargo.toml

# Look for any other protobuf version references
rg -i "protobuf.*version" --type toml --type typescript

Length of output: 401


30-31: Verify protoc command changes

The switch from --rust_out to --rs_out and removal of experimental options represents a significant change in the protocol buffer code generation approach.

Could you confirm if:

  1. The --rs_out flag is the new recommended approach?
  2. The removed experimental options are no longer needed?
✅ Verification successful

Protoc command changes are correct and intentional

The switch to --rs_out is the correct approach as confirmed by:

  • Generated files headers showing rust-protobuf 3.7.1 using --rs_out
  • Matching protobuf-codegen v3.7.1 installation in the build task
  • Successfully generated protocol files in the codebase

The removal of experimental options appears intentional and working correctly with the current toolchain.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check protoc version and help
protoc --version
protoc --help | grep -A 5 "rs_out"

# Look for any documentation or comments about this change
rg -l "rust_out|rs_out" .

Length of output: 380


Script:

#!/bin/bash
# Check if these are generated files and look for any documentation
cat src/substantial/src/protocol/metadata.rs | head -n 10
cat src/substantial/src/protocol/events.rs | head -n 10

# Look for documentation about protoc in markdown files
fd -e md | xargs rg -l "protoc|protocol buffer"

# Check the gen.ts file for comments about the change
cat tools/tasks/gen.ts

Length of output: 1870

tests/metagen/typegraphs/sample/rs/Cargo.toml (1)

7-7: Verify workspace configuration impact.

Switching from workspace to path dependency could lead to version inconsistencies. Please verify:

  1. Why the workspace dependency is being removed
  2. If this change is necessary for the Rust 1.82.0 upgrade
✅ Verification successful

Path dependency is correct due to workspace configuration

The change from workspace to path dependency is necessary because the test directory is explicitly excluded from the workspace in the root Cargo.toml. Both test packages consistently use path dependencies as they cannot use workspace dependencies when excluded.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if the repository has a workspace configuration
fd -t f "Cargo.toml" -x grep -l "workspace.members"

# Check if metagen-client-rs is referenced in other Cargo.toml files
fd -t f "Cargo.toml" -x grep -l "metagen-client"

Length of output: 247


Script:

#!/bin/bash
# Check root Cargo.toml content
echo "=== Root Cargo.toml ==="
cat Cargo.toml

# Check how metagen-client is referenced in other files
echo -e "\n=== src/metagen-client-rs/Cargo.toml ==="
cat src/metagen-client-rs/Cargo.toml

echo -e "\n=== tests/metagen/typegraphs/sample/rs_upload/Cargo.toml ==="
cat tests/metagen/typegraphs/sample/rs_upload/Cargo.toml

Length of output: 7859

src/typegate/src/typegate/hooks.ts (1)

7-7: LGTM!

The import statement is properly typed and follows TypeScript conventions.

tools/installs.ts (1)

12-17: LGTM! Comprehensive Rust configuration for WASM support.

The enhanced Rust configuration properly includes the necessary components and targets for WASM compilation.

Let's verify the Rust configuration:

✅ Verification successful

Configuration verified: Rust setup properly supports WASM compilation

The configuration in tools/installs.ts correctly aligns with:

  • Project's rust-toolchain.toml configuration
  • Multiple WASM build scripts and requirements
  • Documentation and usage patterns across the codebase
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify Rust configuration and WASM targets
# Test: Check if rust-src component and WASM targets are available in the current Rust installation

rustup component list | grep "rust-src"
rustup target list | grep -E "wasm32-(unknown-unknown|wasi)"

Length of output: 296


Script:

#!/bin/bash
# Search for WASM and Rust-related configurations
echo "=== Searching for WASM targets usage ==="
rg -i "wasm32-(unknown-unknown|wasi)" -A 2 -B 2

echo -e "\n=== Searching for rust-src component references ==="
rg -i "rust-src" -A 2 -B 2

echo -e "\n=== Looking for Rust build configurations ==="
fd -e toml -e cargo --exec cat {}

Length of output: 31199

tests/metagen/typegraphs/sample.ts (1)

25-32: LGTM! Well-structured type definitions.

The reformatted struct definitions improve readability while maintaining functionality.

Also applies to: 34-41

tests/metagen/typegraphs/sample/ts/main.ts (2)

25-48: LGTM! Well-structured mutation preparation.

The reformatted mutation preparation improves readability with clear type annotations and argument handling.


134-147: LGTM! Comprehensive test coverage for nested structures.

The new query thoroughly tests all aspects of the nestedComposite structure, including scalar fields, nested structs, and lists.

tests/injection/random_injection_test.ts (2)

20-78: LGTM! Clean formatting improvements.

The reformatting of the test structure enhances readability while maintaining the original functionality.


86-103: LGTM! Well-structured test cases.

The test cases for random lists and variant types are well-organized and properly formatted.

Also applies to: 110-171

tests/metagen/typegraphs/sample/py/main.py (1)

144-169: LGTM! Good test coverage for nested composites.

The new query comprehensively tests nested composite structures with various selection patterns:

  • Scalar-only selections
  • Struct selections
  • Nested struct selections
  • List selections

All using consistent SelectionFlags(select_all=True) for thorough coverage.

src/typegate/src/runtimes/substantial/workflow_worker_manager.ts (1)

130-130: LGTM! Required permission for Deno v2.1.2.

The addition of import: true to worker permissions is necessary for the Deno v2.1.2 upgrade to enable module imports in the worker environment.

✅ Verification successful

Dynamic imports confirmed in worker files

The import: true permission is indeed required as multiple worker files use dynamic imports with toFileUrl, specifically in:

  • src/typegate/src/runtimes/substantial/worker.ts
  • src/typegate/src/runtimes/deno/worker.ts
  • src/typegate/src/runtimes/deno/hooks/worker.ts
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify if any workers are importing modules that would require this permission

# Search for import statements in worker-related files
rg -l "import .+ from" . --type ts --glob "**/worker*.ts"

Length of output: 279


Script:

#!/bin/bash
# Search for dynamic imports in worker-related files
rg "import\s*\(" --type ts --glob "**/worker*.ts" -C 2

# Also check for specific Deno import APIs that would require this permission
rg "Deno\.import" --type ts --glob "**/worker*.ts" -C 2

Length of output: 1409

tests/metagen/typegraphs/sample/rs/main.rs (2)

139-166: Well-structured query implementation with comprehensive test coverage.

The nested composite query implementation demonstrates good practices:

  • Progressive complexity in selections
  • Proper use of the spread operator
  • Clear structure for nested field selections

201-206: LGTM: Consistent output structure.

The integration of res6 into the JSON output maintains the established pattern and naming conventions.

tests/runtimes/wasm_wire/rust/fdk.rs (1)

112-112: Version bump from rc.8 to rc.9.

Clean version update that maintains compatibility checking in the init method.

tests/runtimes/deno/deno_sync_test.ts (1)

279-281: LGTM: Added sync prefix configuration.

Appropriate addition of the sync prefix for the test engine configuration, which aligns with the test's purpose.

tests/e2e/published/published_test.ts (3)

82-86: Improved download function implementation.

The update to use downloadFile with structured parameters improves code clarity and maintainability.


371-379: Enhanced error handling with better diagnostics.

Good addition of error checking and detailed error reporting for typegate readiness.


346-346: Added necessary Deno configuration.

Added workspace configuration for the test environment.

tests/metagen/typegraphs/sample/rs/client.rs (1)

167-222: Well-structured implementation of nested composite functionality.

The implementation follows a clear hierarchical structure with proper separation of concerns:

  • Root level composite structure
  • Nested composite structure
  • List structure handling
src/metagen/src/client_ts/static/mod.ts (1)

110-118: Optimization: Skip processing of non-explicit composite selections.

The added check improves performance by avoiding unnecessary processing when using selectAll flag. This change aligns with similar optimizations in the Python and Rust implementations.

src/metagen-client-rs/src/selection.rs (1)

Line range hint 396-404: Consistent behavior change for composite selections.

The modification to return Self::Skip instead of including all selections:

  • Establishes a more conservative default behavior
  • Reduces unnecessary processing
  • Aligns with similar changes in TypeScript and Python implementations
tests/metagen/typegraphs/sample/ts_upload/client.ts (1)

113-118: Consistent implementation of selectAll optimization.

The implementation:

  • Mirrors the changes in mod.ts
  • Maintains consistency across the codebase
  • Improves performance by skipping unnecessary processing
src/metagen/src/client_py/static/client.py (2)

107-110: LGTM: Consistent variable naming

The change from flags to sub_flags improves code clarity by making it explicit that these flags are for sub-selections.


123-132: LGTM: Enhanced selection handling

The new logic for skipping non-explicit composite selections when using select_all improves the control flow and prevents unnecessary node processing.

tests/metagen/typegraphs/sample/py_upload/client.py (1)

126-134: LGTM: Consistent implementation

The changes maintain consistency with the previous file, implementing the same selection handling improvements.

tests/metagen/typegraphs/sample/ts/client.ts (3)

977-1012: LGTM: Well-structured node metadata implementations

The new node metadata functions are well-organized and follow a consistent pattern, properly defining the structure for nested composites.


1043-1058: LGTM: Clear type definitions

The type definitions for nested composite structures are clear and properly typed.


1184-1191: LGTM: Clean method implementation

The nestedComposite method implementation follows the established pattern and properly integrates with the existing codebase.

import_map.json (3)

3-16: LGTM: Updated to JSR registry

The migration to JSR (jsr:) for standard library imports aligns with modern Deno practices.


17-19: LGTM: AWS SDK version updates

AWS SDK dependencies have been updated to version 3.723.0, maintaining security and compatibility.


45-66: LGTM: Node.js compatibility layer

The addition of Node.js core module imports supports Deno's Node.js compatibility layer, which is essential for the Deno 2.1.2 update.

.ghjk/lock.json (2)

746-793: Verify Rust toolchain configuration.

The Rust toolchain has been updated with new configuration. Ensure all components and targets are necessary.

✅ Verification successful

Rust toolchain configuration is correctly configured ✓

All components and targets specified in the configuration are actively used in the codebase:

  • wasm32-unknown-unknown target is used for WASM components in runtime tests
  • wasm32-wasi target is used in metagen and examples
  • rust-src component is required for IDE integration
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify Rust components and targets usage
# Check if all configured components and targets are actually used

# Search for WASM target usage
rg 'wasm32-(unknown-unknown|wasi)'

# Search for rust-src component usage
rg 'rust-src'

Length of output: 1500


734-745: Verify compatibility with new Deno versions.

New Deno versions have been added. Ensure all Deno-dependent code is compatible with these versions.

✅ Verification successful

No compatibility issues detected with Deno version updates

The codebase uses only stable Deno APIs and no version-specific features. The version updates (v2.0.6 to v2.1.2) are minor and should maintain backward compatibility.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for Deno version-specific features in use
# Look for usage of Deno APIs that might have changed between versions

# Search for Deno API usage
rg --type ts --type js 'Deno\.' -A 2

Length of output: 40168

tests/metagen/typegraphs/sample/py/client.py (4)

126-134: LGTM: Improved handling of select_all flag.

The new logic correctly handles the select_all flag for non-explicit composite selections.


951-993: LGTM: Well-structured nested composite type definitions.

The new static methods for nested composite types are well-organized and follow the established pattern. The hierarchy is clear and maintainable.


Line range hint 999-1178: LGTM: Well-defined type hierarchy.

The TypedDict definitions are properly structured with correct typing and follow Python's type hinting best practices. The use of total=False makes the types more flexible.


1183-1190: LGTM: Consistent implementation of new functionality.

The type mapping and new method implementation are consistent with the existing codebase patterns and properly handle the new nested composite types.

Also applies to: 1288-1298

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 58673a1 and a15fc55.

📒 Files selected for processing (5)
  • .ghjk/lock.json (16 hunks)
  • tests/runtimes/deno/ts/math.ts (1 hunks)
  • tools/Dockerfile (2 hunks)
  • tools/Dockerfile.dockerignore (1 hunks)
  • tools/cross.Dockerfile (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • tools/Dockerfile.dockerignore
  • tools/Dockerfile
  • tests/runtimes/deno/ts/math.ts
⏰ Context from checks skipped due to timeout of 90000ms (4)
  • GitHub Check: lint-compat (macos-14, aarch64-apple-darwin, false)
  • GitHub Check: bulid-docker (linux/amd64, custom-ubuntu-large)
  • GitHub Check: lint-compat (macos-13, x86_64-apple-darwin, false)
  • GitHub Check: pre-commit
🔇 Additional comments (7)
tools/cross.Dockerfile (2)

40-41: LGTM! Good URL update.

Updating the installation script URL from raw.github.com to raw.githubusercontent.com is correct as it uses the official domain for raw content.


Line range hint 5-5: Consider using a specific version tag instead of :main

Using the :main tag for the base image (ghcr.io/cross-rs/$CROSS_TARGET:main) could lead to reproducibility issues as the image content may change unexpectedly. Consider using a specific version tag to ensure build reproducibility.

Run this script to check available tags:

✅ Verification successful

The base image change is justified despite using the :main tag

While using a specific version tag would typically be recommended for better reproducibility, moving away from an EOL Ubuntu 16 base image with outdated libraries is a significant improvement for security and functionality. The ghcr.io/cross-rs/$CROSS_TARGET:main image, even with the floating tag, is a better choice in this case.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check available tags for the cross-rs images
# Note: This assumes the CROSS_TARGET is typically something like x86_64-unknown-linux-gnu

# List available tags
gh api \
  -H "Accept: application/vnd.github+json" \
  /orgs/cross-rs/packages/container/cross-rs%2Fx86_64-unknown-linux-gnu/versions \
  --jq '.[].metadata.container.tags[]'

Length of output: 376


Script:

#!/bin/bash
# Search for any cross-rs related Dockerfile references
rg -l "cross-rs" --type dockerfile

# Search for any CROSS_TARGET usage in Dockerfiles
rg "CROSS_TARGET" --type dockerfile -C 2

# List all Dockerfiles to analyze the context
fd Dockerfile

Length of output: 231


Script:

#!/bin/bash
echo "=== tools/cross.Dockerfile ==="
cat tools/cross.Dockerfile

echo -e "\n=== tools/Dockerfile ==="
cat tools/Dockerfile

Length of output: 6020

.ghjk/lock.json (5)

740-745: LGTM! Deno updated to v2.1.2.

The update to Deno v2.1.2 aligns with the PR objectives.


Line range hint 854-1076: LGTM! Environment sets consistently updated.

All environment sets have been systematically updated to use the new version identifiers while maintaining the existing structure.


2143-2143: LGTM! Default environment updated.

The default environment identifier has been updated consistently.


Line range hint 2150-2161: LGTM! Deno configuration properly structured.

The blackboard configuration for Deno:

  • Correctly specifies version 2.1.2
  • Supports multiple platforms (Linux, Darwin, Windows)
  • Maintains proper port configuration

746-794: LGTM! Rust updated to 1.82.0 with enhanced WebAssembly support.

The Rust update includes:

  • Version bump to 1.82.0
  • Profile set to "default"
  • Added "rust-src" component
  • Added WebAssembly targets

Run the following script to verify the Rust configuration:

✅ Verification successful

Verified: Rust 1.82.0 configuration is consistent with proper WebAssembly support

  • All Rust instances are correctly updated to version 1.82.0
  • WebAssembly targets (wasm32-unknown-unknown, wasm32-wasi) are properly configured
  • The rust-src component is present where required
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify Rust toolchain configuration

# Check if rust-src component is available
rustup component list | grep "rust-src"

# Verify WebAssembly targets are installed
rustup target list | grep -E "wasm32-(unknown-unknown|wasi)"

Length of output: 296


Script:

#!/bin/bash
# Search for all Rust configurations in lock.json
echo "=== Rust Configurations ==="
rg -A 20 '"version": "1\.82\.0"' .ghjk/lock.json

echo -e "\n=== Version Consistency Check ==="
# Extract and compare all Rust versions
rg '"version":.+"1\.82\.0"' .ghjk/lock.json

echo -e "\n=== WebAssembly Configurations ==="
# Find all WebAssembly target configurations
rg -B 5 -A 5 '"wasm32-' .ghjk/lock.json

Length of output: 5116

Yohe-Am and others added 9 commits January 15, 2025 07:02
<!--
Pull requests are squashed and merged using:
- their title as the commit message
- their description as the commit body

Having a good title and description is important for the users to get
readable changelog.
-->

<!-- 1. Explain WHAT the change is about -->

- Closes
[MET-786](https://linear.app/metatypedev/issue/MET-786/typescript-client-selectall-infinite-recursion).

<!-- 3. Explain HOW users should update their code -->

---

- [x] The change comes with new or modified tests
- [ ] Hard-to-understand functions have explanatory comments
- [ ] End-user documentation is updated to reflect the change

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

- **New Features**
- Added nested composite structure support across multiple client
implementations
	- Enhanced selection handling for composite queries
	- Expanded type definitions for more complex data representations

- **Bug Fixes**
	- Improved selection processing logic in client implementations
	- Updated version compatibility for SDK imports

- **Chores**
	- Updated package dependencies to newer SDK versions
	- Reformatted and improved code readability across multiple files

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Update protobuf-codegen and protobuff to latest (3.7.1)

---

- [ ] The change comes with new or modified tests
- [ ] Hard-to-understand functions have explanatory comments
- [ ] End-user documentation is updated to reflect the change

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

- **Dependency Updates**
  - Updated `protobuf` library from version 3.5.1 to 3.7.1
  - Updated `protobuf-codegen` crate to version 3.7.1

- **Build Configuration**
- Modified protobuf compiler output option from `--rust_out` to
`--rs_out`
  - Simplified protobuf code generation command

- **Compatibility**
  - Ensured compatibility with latest protobuf runtime version

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
<!--
Pull requests are squashed and merged using:
- their title as the commit message
- their description as the commit body

Having a good title and description is important for the users to get
readable changelog.
-->

<!-- 1. Explain WHAT the change is about -->

- Add a self-hosted json schema that will be referenced to
[schemastore](https://www.schemastore.org/json/), part of
[MET-798](https://linear.app/metatypedev/issue/MET-798/metatype-schema-for-ide-support).

<!-- 3. Explain HOW users should update their code -->

---

- [x] The change comes with new or modified tests
- [ ] Hard-to-understand functions have explanatory comments
- [ ] End-user documentation is updated to reflect the change

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

- **New Features**
- Added comprehensive configuration schema validation for Metatype
system configuration files.
- Implemented JSON schema testing to ensure configuration integrity
across multiple YAML files.

- **Tests**
- Introduced new test suite for validating configuration schema using
Ajv JSON schema validator.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (1)
.github/workflows/tests.yml (1)

289-290: ⚠️ Potential issue

Fix shell script issues in the test-full job.

The shell script issues are identical to those in the test-website job.

Apply the same fixes as suggested in the test-website job comment.

🧰 Tools
🪛 actionlint (1.7.4)

289-289: shellcheck reported issue in this script: SC2088:warning:1:7: Tilde does not expand in quotes. Use $HOME

(shellcheck)


289-289: shellcheck reported issue in this script: SC2086:info:1:24: Double quote to prevent globbing and word splitting

(shellcheck)


290-290: shellcheck reported issue in this script: SC2086:info:1:47: Double quote to prevent globbing and word splitting

(shellcheck)

🧹 Nitpick comments (1)
.github/workflows/tests.yml (1)

91-96: Consider creating a reusable composite action.

The installation steps are duplicated across multiple jobs. Consider creating a composite action to encapsulate these steps:

# .github/actions/install-ghjk/action.yml
name: 'Install GHJK'
description: 'Installs GHJK with proper environment setup'
runs:
  using: "composite"
  steps:
    - shell: bash
      run: curl -fsSL "https://raw.githubusercontent.com/metatypedev/ghjk/$GHJK_VERSION/install.sh" | bash
    - shell: bash
      run: |
        echo "$HOME/.local/bin" >> "$GITHUB_PATH"
        echo "BASH_ENV=$HOME/.local/share/ghjk/env.sh" >> "$GITHUB_ENV"
    - shell: bash
      run: ghjk envs cook
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3222078 and 2383b82.

⛔ Files ignored due to path filters (2)
  • Cargo.lock is excluded by !**/*.lock
  • deno.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • .github/workflows/tests.yml (5 hunks)
  • tests/tools/schema_test.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/tools/schema_test.ts
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/tests.yml

176-176: shellcheck reported issue in this script: SC2088:warning:1:7: Tilde does not expand in quotes. Use $HOME

(shellcheck)


176-176: shellcheck reported issue in this script: SC2086:info:1:24: Double quote to prevent globbing and word splitting

(shellcheck)


177-177: shellcheck reported issue in this script: SC2086:info:1:47: Double quote to prevent globbing and word splitting

(shellcheck)


289-289: shellcheck reported issue in this script: SC2088:warning:1:7: Tilde does not expand in quotes. Use $HOME

(shellcheck)


289-289: shellcheck reported issue in this script: SC2086:info:1:24: Double quote to prevent globbing and word splitting

(shellcheck)


290-290: shellcheck reported issue in this script: SC2086:info:1:47: Double quote to prevent globbing and word splitting

(shellcheck)

🪛 yamllint (1.35.1)
.github/workflows/tests.yml

[error] 173-173: trailing spaces

(trailing-spaces)


[error] 284-284: trailing spaces

(trailing-spaces)

⏰ Context from checks skipped due to timeout of 90000ms (7)
  • GitHub Check: bulid-docker (linux/arm64, custom-arm)
  • GitHub Check: lint-compat (macos-14, aarch64-apple-darwin, false)
  • GitHub Check: bulid-docker (linux/amd64, custom-ubuntu-large)
  • GitHub Check: lint-compat (macos-13, x86_64-apple-darwin, false)
  • GitHub Check: test-website
  • GitHub Check: test-full
  • GitHub Check: pre-commit
🔇 Additional comments (2)
.github/workflows/tests.yml (2)

176-177: ⚠️ Potential issue

Fix shell script issues in the test-website job.

The current implementation has shell script issues that could cause problems:

  • Tilde (~) doesn't expand in quotes
  • Missing quotes around variables could cause word splitting

Apply these fixes:

-      - run: echo "~/.local/bin" >> $GITHUB_PATH
-      - run: echo "BASH_ENV=~/.local/share/ghjk/env.sh" >> $GITHUB_ENV
+      - run: echo "$HOME/.local/bin" >> "$GITHUB_PATH"
+      - run: echo "BASH_ENV=$HOME/.local/share/ghjk/env.sh" >> "$GITHUB_ENV"

Likely invalid or redundant comment.

🧰 Tools
🪛 actionlint (1.7.4)

176-176: shellcheck reported issue in this script: SC2088:warning:1:7: Tilde does not expand in quotes. Use $HOME

(shellcheck)


176-176: shellcheck reported issue in this script: SC2086:info:1:24: Double quote to prevent globbing and word splitting

(shellcheck)


177-177: shellcheck reported issue in this script: SC2086:info:1:47: Double quote to prevent globbing and word splitting

(shellcheck)


32-32: Verify the impact of disabling GHJK logging.

The removal of debug logging might impact troubleshooting capabilities in CI. Consider keeping it enabled or documenting alternative debugging methods.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
tools/jsr/jsr-gen.ts (1)

Line range hint 31-73: Consider these maintainability improvements.

While the code works well, consider these enhancements:

  1. Extract the file extension regex pattern into a constant
  2. Add type definition for the configuration object
  3. Move magic strings in exclude arrays to constants

Here's how you could implement these improvements:

// At the top of the file
const SUPPORTED_EXTENSIONS = /\.(ts|js|mjs)$/;
const EXCLUDED_FILES = ["index.ts"] as const;
const EXCLUDED_LINT_RULES = ["no-external-import"] as const;

// Type for the configuration
type DenoConfig = {
  name: string;
  version: string;
  publish: {
    exclude: string[];
  };
  lint: {
    rules: {
      exclude: string[];
    };
  };
  exports: Record<string, string>;
};

// Then use these in your code:
if (SUPPORTED_EXTENSIONS.test(path)) {
  // ... rest of the code

// And type the configuration:
const config: DenoConfig = {
  name: SDK_PACKAGE_NAME_TS,
  version: CURRENT_VERSION,
  publish: {
    exclude: ["!src/gen", "!LICENSE.md", "!README.md"],
  },
  lint: {
    rules: {
      exclude: EXCLUDED_LINT_RULES,
    },
  },
  exports: jsrExports,
};

Deno.writeTextFileSync(
  join(denoSdkDir, "deno.json"),
  JSON.stringify(config, null, 2) + "\n"
);
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2383b82 and 456629b.

⛔ Files ignored due to path filters (1)
  • deno.lock is excluded by !**/*.lock
📒 Files selected for processing (5)
  • import_map.json (1 hunks)
  • src/typegate/src/transports/graphql/typegraph.ts (0 hunks)
  • src/typegate/src/typegraph/versions.ts (1 hunks)
  • tests/e2e/published/utils.ts (0 hunks)
  • tools/jsr/jsr-gen.ts (1 hunks)
💤 Files with no reviewable changes (2)
  • tests/e2e/published/utils.ts
  • src/typegate/src/transports/graphql/typegraph.ts
✅ Files skipped from review due to trivial changes (1)
  • src/typegate/src/typegraph/versions.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • import_map.json
⏰ Context from checks skipped due to timeout of 90000ms (7)
  • GitHub Check: lint-compat (macos-14, aarch64-apple-darwin, false)
  • GitHub Check: bulid-docker (linux/arm64, custom-arm)
  • GitHub Check: lint-compat (macos-13, x86_64-apple-darwin, false)
  • GitHub Check: bulid-docker (linux/amd64, custom-ubuntu-large)
  • GitHub Check: test-full
  • GitHub Check: test-website
  • GitHub Check: pre-commit
🔇 Additional comments (1)
tools/jsr/jsr-gen.ts (1)

5-5: LGTM! Good improvement in file existence checking.

The removal of existsSync in favor of $.path(path).exists() is a good change that aligns with Deno's idiomatic patterns.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (2)
.github/workflows/tests.yml (2)

288-289: ⚠️ Potential issue

Use $HOME instead of tilde in GitHub Actions.

GitHub Actions may not expand the tilde character correctly in all environments. Use $HOME instead for consistent behavior.

-      - run: echo "~/.local/bin" >> $GITHUB_PATH
-      - run: echo "BASH_ENV=~/.local/share/ghjk/env.sh" >> $GITHUB_ENV
+      - run: echo "$HOME/.local/bin" >> "$GITHUB_PATH"
+      - run: echo "BASH_ENV=$HOME/.local/share/ghjk/env.sh" >> "$GITHUB_ENV"

175-176: ⚠️ Potential issue

Use $HOME instead of tilde in GitHub Actions.

GitHub Actions may not expand the tilde character correctly in all environments. Use $HOME instead for consistent behavior.

-      - run: echo "~/.local/bin" >> $GITHUB_PATH
-      - run: echo "BASH_ENV=~/.local/share/ghjk/env.sh" >> $GITHUB_ENV
+      - run: echo "$HOME/.local/bin" >> "$GITHUB_PATH"
+      - run: echo "BASH_ENV=$HOME/.local/share/ghjk/env.sh" >> "$GITHUB_ENV"
🧹 Nitpick comments (2)
src/typegate/engine/Cargo.toml (1)

58-58: Add workspace dependency for deno_error

This change integrates deno_error into the workspace, aligning with the overall dependency management improvements to support the updated Deno runtime. Please verify that this addition is compatible with Deno v2.1.2 and that any downstream modules using error handling from this crate have been updated accordingly.

.github/workflows/release.yml (1)

37-37: Remove trailing whitespace in YAML file.

There are trailing spaces on these lines that should be removed.

-      
+

Also applies to: 185-185, 324-324

🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 37-37: trailing spaces

(trailing-spaces)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 456629b and 8dfe626.

⛔ Files ignored due to path filters (2)
  • Cargo.lock is excluded by !**/*.lock
  • deno.lock is excluded by !**/*.lock
📒 Files selected for processing (38)
  • .ghjk/lock.json (10 hunks)
  • .github/workflows/release.yml (5 hunks)
  • .github/workflows/tests.yml (5 hunks)
  • Cargo.toml (3 hunks)
  • deno.jsonc (2 hunks)
  • examples/deno.jsonc (0 hunks)
  • import_map.json (1 hunks)
  • rust-toolchain.toml (1 hunks)
  • src/metagen-client-rs/Cargo.toml (2 hunks)
  • src/metagen-client-rs/src/common.rs (2 hunks)
  • src/mt_deno/src/lib.rs (9 hunks)
  • src/typegate/engine/00_runtime.js (1 hunks)
  • src/typegate/engine/Cargo.toml (1 hunks)
  • src/typegate/engine/bindings.ts (22 hunks)
  • src/typegate/engine/runtime.d.ts (1 hunks)
  • src/typegate/engine/runtime.js (1 hunks)
  • src/typegate/engine/src/error.rs (1 hunks)
  • src/typegate/engine/src/ext.rs (3 hunks)
  • src/typegate/engine/src/lib.rs (5 hunks)
  • src/typegate/engine/src/py_validation.rs (2 hunks)
  • src/typegate/engine/src/runtimes/grpc.rs (6 hunks)
  • src/typegate/engine/src/runtimes/prisma.rs (9 hunks)
  • src/typegate/engine/src/runtimes/substantial.rs (29 hunks)
  • src/typegate/engine/src/runtimes/temporal.rs (14 hunks)
  • src/typegate/engine/src/runtimes/wasm.rs (3 hunks)
  • src/typegate/engine/src/runtimes/wasm/conversion.rs (1 hunks)
  • src/typegate/engine/src/runtimes/wit_wire.rs (2 hunks)
  • src/typegate/engine/src/typegraph.rs (1 hunks)
  • src/typegate/src/engine/planner/args.ts (1 hunks)
  • src/typegate/src/engine/planner/policies.ts (1 hunks)
  • src/typegate/src/runtimes/patterns/worker_manager/deno.ts (1 hunks)
  • src/typegate/src/runtimes/python/hooks/mod.ts (2 hunks)
  • src/typegate/src/runtimes/substantial.ts (1 hunks)
  • src/typegate/src/runtimes/substantial/agent.ts (1 hunks)
  • src/typegate/src/runtimes/wit_wire/mod.ts (1 hunks)
  • src/typegate/src/services/artifact_service.ts (2 hunks)
  • src/typegate/src/services/graphql_service.ts (2 hunks)
  • src/typegate/src/typegate/mod.ts (1 hunks)
💤 Files with no reviewable changes (1)
  • examples/deno.jsonc
✅ Files skipped from review due to trivial changes (1)
  • src/typegate/src/runtimes/wit_wire/mod.ts
🚧 Files skipped from review as they are similar to previous changes (13)
  • src/typegate/src/engine/planner/policies.ts
  • src/typegate/src/runtimes/python/hooks/mod.ts
  • rust-toolchain.toml
  • deno.jsonc
  • src/typegate/src/engine/planner/args.ts
  • src/metagen-client-rs/Cargo.toml
  • src/typegate/src/services/artifact_service.ts
  • src/typegate/src/typegate/mod.ts
  • src/typegate/engine/src/lib.rs
  • src/typegate/engine/src/ext.rs
  • src/mt_deno/src/lib.rs
  • Cargo.toml
  • src/typegate/engine/bindings.ts
🧰 Additional context used
🧬 Code Definitions (8)
src/typegate/engine/00_runtime.js (2)
src/typegate/engine/runtime.d.ts (1) (1)
  • Meta (4-4)
src/typegate/engine/runtime.js (2) (2)
  • Meta (10-10)
  • Meta (10-10)
src/typegate/engine/runtime.js (2)
src/typegate/engine/00_runtime.js (1) (1)
  • Meta (31-90)
src/typegate/engine/runtime.d.ts (1) (1)
  • Meta (4-4)
src/typegate/engine/src/py_validation.rs (1)
src/typegate/engine/src/error.rs (1) (1)
  • map (17-19)
src/typegate/engine/src/typegraph.rs (3)
src/typegate/engine/src/runtimes/wasm.rs (1) (1)
  • serde_json (35-35)
src/typegate/engine/src/runtimes/wasm/conversion.rs (1) (1)
  • serde_json (212-212)
src/typegate/engine/src/error.rs (1) (1)
  • map (17-19)
src/typegate/engine/src/runtimes/grpc.rs (4)
src/typegate/engine/src/error.rs (1) (1)
  • map (17-19)
src/typegate/engine/src/runtimes/prisma.rs (7) (7)
  • state (59-59)
  • state (78-78)
  • state (109-109)
  • state (159-159)
  • state (190-190)
  • state (223-223)
  • state (242-242)
src/typegate/engine/src/runtimes/substantial.rs (9) (9)
  • state (63-63)
  • state (92-92)
  • state (125-125)
  • state (160-160)
  • state (187-187)
  • state (216-216)
  • state (244-244)
  • state (272-272)
  • state (293-293)
src/typegate/engine/src/runtimes/wit_wire.rs (1) (1)
  • ctx (168-170)
src/typegate/engine/src/runtimes/prisma.rs (3)
src/typegate/engine/src/error.rs (1) (1)
  • map (17-19)
src/typegate/engine/src/runtimes/prisma/engine.rs (1) (1)
  • query (33-51)
src/typegate/engine/src/runtimes/prisma/migration.rs (4) (4)
  • diff (234-278)
  • new (80-87)
  • new (307-342)
  • new (352-361)
src/typegate/engine/src/runtimes/substantial.rs (4)
src/typegate/engine/runtime.d.ts (3) (3)
  • CreateOrGetOutput (335-337)
  • Run (325-328)
  • Operation (323-323)
src/typegate/engine/src/error.rs (1) (1)
  • map (17-19)
src/typegate/engine/src/runtimes/prisma.rs (7) (7)
  • state (59-59)
  • state (78-78)
  • state (109-109)
  • state (159-159)
  • state (190-190)
  • state (223-223)
  • state (242-242)
src/typegate/engine/src/runtimes/temporal.rs (5) (5)
  • state (50-50)
  • state (90-90)
  • state (146-146)
  • state (191-191)
  • state (251-251)
src/typegate/src/services/graphql_service.ts (2)
src/typegate/src/services/auth/routes/validate.ts (1) (1)
  • badRequest (7-9)
src/typegate/src/services/responses.ts (2) (2)
  • badRequest (57-61)
  • jsonError (32-55)
🪛 YAMLlint (1.35.1)
.github/workflows/tests.yml

[error] 172-172: trailing spaces

(trailing-spaces)


[error] 283-283: trailing spaces

(trailing-spaces)

.github/workflows/release.yml

[error] 37-37: trailing spaces

(trailing-spaces)


[error] 185-185: trailing spaces

(trailing-spaces)


[error] 324-324: trailing spaces

(trailing-spaces)

🪛 Biome (1.9.4)
src/typegate/src/services/graphql_service.ts

[error] 133-133: expected , but instead found :

Remove :

(parse)


[error] 133-133: expected , but instead found (

Remove (

(parse)


[error] 134-134: expected , but instead found :

Remove :

(parse)


[error] 135-135: expected , but instead found headers

Remove headers

(parse)


[error] 136-136: Expected an expression but instead found '}'.

Expected an expression here.

(parse)


[error] 136-136: Expected a statement but instead found ')'.

Expected a statement here.

(parse)


[error] 137-137: Expected a statement but instead found 'else'.

Expected a statement here.

(parse)

🔇 Additional comments (76)
src/typegate/src/services/graphql_service.ts (2)

55-59: Error variable renaming for consistency

The change from e to err for the caught error variable improves code consistency and readability. This convention is better aligned with standard practices for error handling.


123-145: Error variable renaming for consistency

The consistent renaming of error variable from e to err throughout this catch block aligns with the changes made earlier in the file, improving overall code consistency.

🧰 Tools
🪛 Biome (1.9.4)

[error] 133-133: expected , but instead found :

Remove :

(parse)


[error] 133-133: expected , but instead found (

Remove (

(parse)


[error] 134-134: expected , but instead found :

Remove :

(parse)


[error] 135-135: expected , but instead found headers

Remove headers

(parse)


[error] 136-136: Expected an expression but instead found '}'.

Expected an expression here.

(parse)


[error] 136-136: Expected a statement but instead found ')'.

Expected a statement here.

(parse)


[error] 137-137: Expected a statement but instead found 'else'.

Expected a statement here.

(parse)


[error] 137-145: This block statement doesn't serve any purpose and can be safely removed.

Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.

(lint/complexity/noUselessLoneBlockStatements)

src/typegate/src/runtimes/patterns/worker_manager/deno.ts (1)

37-37: Enhanced worker permissions by enabling imports

This change enables the import permission for Deno workers, allowing them to dynamically import modules. This is likely a necessary update to maintain compatibility with Deno v2.1.2, which might have stricter default permission settings.

The permission is appropriately placed in the non-overridable permissions section, maintaining the security boundary between typegraphs while enabling required functionality.

src/metagen-client-rs/src/common.rs (2)

243-245: Improved memory efficiency with heap allocation for error responses

The error handling has been updated to box the GraphQLResponse object when creating a RequestFailed error. This change helps reduce the memory footprint of the GraphQLRequestError enum variants.


292-292: Optimized error enum memory layout

The RequestFailed variant now stores the response as a boxed value (Box<GraphQLResponse>) rather than inline, which optimizes the enum's memory layout. This is a good practice in Rust when dealing with variants that contain larger structs.

.ghjk/lock.json (6)

728-733: Remove unnecessary intermediate Deno version.

The v2.0.6 entry appears to be an unnecessary intermediate version since we're updating directly to v2.1.2. Having both versions could lead to confusion.


734-739: Good addition of Deno v2.1.2.

This implements the primary objective of the PR to update Deno to v2.1.2.


740-760: Rust update looks good.

Updating Rust to v1.82.0 aligns with the PR objectives. The configuration is properly structured.


761-788: Enhanced Rust configuration with important components and targets.

The addition of a Rust configuration with the "default" profile and essential components like "rust-src" and WebAssembly targets will improve development capabilities.


789-812: Consider if all these Deno versions are necessary.

The PR adds multiple additional Deno versions (v2.1.4, v2.1.5, v2.1.6, v2.1.10) beyond the target v2.1.2. Consider if all these intermediate versions are needed or if they might cause version management confusion.


2338-2381: Appropriate Rust configuration in blackboard section.

The updated Rust configuration properly reflects the changes to the version, profile, and additional components and targets.

src/typegate/engine/00_runtime.js (1)

92-92:

❓ Verification inconclusive

Global variable renamed to provide better isolation.

Changed from globalThis.Meta to globalThis.____Meta to create a more isolated namespace for the Meta object. This change works in conjunction with the explicit export of the Meta constant in runtime.js, improving the module structure.

Ensure this change is propagated correctly to all files that might have been directly accessing globalThis.Meta.


🏁 Script executed:

#!/bin/bash
# Search for direct references to globalThis.Meta that might need to be updated
rg "globalThis\.Meta" --type js --type ts

Length of output: 43


Ensure all global references to Meta are updated for better namespace isolation

The change renaming from globalThis.Meta to globalThis.____Meta in src/typegate/engine/00_runtime.js (line 92) is intended to improve isolation and align with the exported Meta constant. The repository search for globalThis.Meta produced no matches, suggesting there are no direct static references remaining. However, the lack of results may be due to limitations in the search or indirect (dynamic) usages. Please perform a manual verification to ensure that no file—especially those employing dynamic property access—is still referencing globalThis.Meta.

  • File to check: src/typegate/engine/00_runtime.js (line 92)
  • Diff snippet:
    globalThis.____Meta = Meta;
  • Next step: Manually verify the entire codebase for any unintended reliance on globalThis.Meta, particularly in cases where dynamic property access might bypass static search tools.
src/typegate/engine/runtime.d.ts (1)

4-4: Export declaration of Meta improves modular usage.

The change from a global declaration to an exported constant improves modularity by making Meta explicitly importable, which is a better practice for maintainable code. This approach provides clearer boundaries for the Meta interface.

src/typegate/engine/src/runtimes/wasm/conversion.rs (1)

4-8: Updated imports to use interlude module.

The change replaces the direct import of Context from anyhow with a more comprehensive import from the local interlude module. This is part of the broader effort to standardize error handling with the new OpErr type across the codebase.

src/typegate/engine/runtime.js (1)

6-10: Added proper export with type annotations for Meta.

The addition of JSDoc type annotation and explicit export for Meta improves type safety and clarity. This aligns with the TypeScript declaration in runtime.d.ts, making it easier for consumers to understand the interface.

src/typegate/src/runtimes/substantial.ts (1)

14-14: Updated imports to include Meta for workflow operations.

The import now explicitly marks Backend as a type and adds Meta, which is needed for the workflow operations in #stopResolver and #linkerResolver methods. This improves type safety while providing access to necessary functionality.

.github/workflows/release.yml (4)

61-61: LGTM! Updated prerelease condition to include 'rc' tags.

The condition has been updated to include 'rc' tagged versions as prereleases alongside 'dev' versions, which is a good improvement for versioning practices.


106-109: Verify the installation script URL domain across all instances.

Same issue with using raw.github.com instead of raw.githubusercontent.com.

-      - run: curl -fsSL "https://raw.github.com/metatypedev/ghjk/$GHJK_VERSION/install.sh" | bash
+      - run: curl -fsSL "https://raw.githubusercontent.com/metatypedev/ghjk/$GHJK_VERSION/install.sh" | bash

186-190: Verify the installation script URL domain across all instances.

Same issue with using raw.github.com instead of raw.githubusercontent.com.

-      - run: curl -fsSL "https://raw.github.com/metatypedev/ghjk/$GHJK_VERSION/install.sh" | bash
+      - run: curl -fsSL "https://raw.githubusercontent.com/metatypedev/ghjk/$GHJK_VERSION/install.sh" | bash

326-329: Verify the installation script URL domain across all instances.

Same issue with using raw.github.com instead of raw.githubusercontent.com.

-      - run: curl -fsSL "https://raw.github.com/metatypedev/ghjk/$GHJK_VERSION/install.sh" | bash
+      - run: curl -fsSL "https://raw.githubusercontent.com/metatypedev/ghjk/$GHJK_VERSION/install.sh" | bash
.github/workflows/tests.yml (4)

31-31: LGTM! Commented out the GHJK_LOG debug variable.

Appropriate change to disable debug logging in the CI environment.


92-95: Verify the installation script URL domain for better security.

The installation command is correctly quoted, but it's using raw.github.com which should be changed to raw.githubusercontent.com for more reliable access.

-      - run: curl -fsSL "https://raw.github.com/metatypedev/ghjk/$GHJK_VERSION/install.sh" | bash
+      - run: curl -fsSL "https://raw.githubusercontent.com/metatypedev/ghjk/$GHJK_VERSION/install.sh" | bash

295-295: Good addition of set -x for better debugging.

Adding set -x to the shell script is beneficial for debugging as it will display each command as it's executed.


172-172: Remove trailing whitespace in YAML file.

There are trailing spaces on these lines that should be removed.

-      
+

Also applies to: 283-283

🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 172-172: trailing spaces

(trailing-spaces)

src/typegate/src/runtimes/substantial/agent.ts (1)

4-11: LGTM! Improved TypeScript import organization.

Good refactoring to explicitly separate type imports from value imports, which aligns with TypeScript best practices and creates clearer boundaries between types and runtime values.

src/typegate/engine/src/py_validation.rs (2)

4-4: LGTM! Added the interlude module import.

Adding the interlude module import provides access to common utilities and types, which aligns with the codebase's conventions.


22-22: Improved error handling with OpErr type.

The function now returns Result<(), OpErr> instead of Result<()> and uses map_err(OpErr::map()) for explicit error handling. This is part of a systematic error handling refactoring across the codebase, improving consistency and error reporting.

Also applies to: 24-24

src/typegate/engine/src/runtimes/wasm.rs (6)

25-25: Updated return type for explicit error handling using OpErr.

The function signature now explicitly specifies OpErr as the return error type, which aligns with the systematic approach to error handling across the codebase.


29-29: Consistent error transformation using anyhow.

Error handling pattern converted to use the .into() conversion to OpErr, maintaining consistency with other error handling in the codebase.


42-42: Improved error handling for file operations.

Using map_err(OpErr::map()) provides a cleaner and more consistent approach to error handling for file operations compared to the previous implementation.


46-48: Standardized error handling for linker instantiation.

The linker instantiation error handling now uses the OpErr::map() pattern, ensuring consistent error types throughout the function.


84-85: Consistent error transformation for value conversion and serialization.

Both the value conversion and JSON serialization now use the same error handling pattern with OpErr::map(), improving consistency and error reporting.


87-87: Updated error handling for function call failures.

Error handling for function calls now returns an OpErr using .into(), completing the consistent error handling approach throughout the function.

src/typegate/engine/src/typegraph.rs (4)

11-11: Updated return type for explicit error handling.

The function signature now explicitly specifies OpErr as the return error type, aligning with the codebase-wide error handling improvements.


12-13: Improved error handling for JSON operations.

Both deserialization and serialization now use the OpErr::map() pattern, ensuring consistent error types and better error reporting.


20-20: Updated return type for validation function.

The function now explicitly returns Result<(), OpErr> instead of Result<()>, clarifying the error type and aligning with the error handling pattern used throughout the codebase.


21-21: Standardized error handling for deserialization.

The deserialization error handling now uses map_err(OpErr::map()), ensuring consistent error types and improving error clarity.

src/typegate/engine/src/runtimes/wit_wire.rs (4)

342-342: Enhanced error handling with Deno JS error integration.

The WitWireInitError enum now derives deno_error::JsError, which allows for better integration with Deno's error handling system, consistent with the Deno v2.1.2 update.


344-356: Expanded error variants with class annotations.

The enum now includes:

  1. New #[class(generic)] attributes for JS error classification
  2. Additional error variants for better error reporting:
    • UnexpectedMat for unexpected material information
    • Other for generic errors
    • ModuleErr for WebAssembly module errors

This provides more specific error reporting, making debugging easier.


443-443: Enhanced error handling for WitWireHandleError.

Similar to WitWireInitError, this enum now derives deno_error::JsError for better integration with Deno's error handling system.


445-450: Added specific error variants for handle operations.

The enum now includes:

  1. #[class(generic)] attributes for JS error classification
  2. New error variants:
    • InstanceNotFound with a descriptive id field
    • ModuleErr for WebAssembly module errors

This improves error specificity and reporting capabilities.

src/typegate/engine/src/runtimes/grpc.rs (5)

150-151: Updated return type and error handling for gRPC registration.

The function now:

  1. Returns Result<(), OpErr> instead of Result<()>, explicitly specifying the error type
  2. Uses map_err(OpErr::map()) for consistent error transformation

This aligns with the codebase-wide error handling improvements.


170-170: Updated return type for gRPC unregistration.

The function's return type changed to Result<(), OpErr>, explicitly specifying the error type and improving consistency.


173-174: Enhanced error handling for client removal failures.

The error handling now:

  1. Retains the descriptive context message
  2. Uses map_err(OpErr::map()) to transform the error into the standard OpErr type

This ensures consistent error reporting throughout the codebase.


192-192: Updated return type for gRPC method calls.

The function now returns Result<String, OpErr> instead of Result<String>, explicitly specifying the error type and aligning with the error handling pattern used throughout the codebase.


201-229: Standardized error handling throughout the gRPC call flow.

Multiple error handling points have been updated to use map_err(OpErr::map()):

  1. Client retrieval (line 202)
  2. File descriptor operations (line 205)
  3. Method descriptor retrieval (line 208)
  4. Request creation (line 213)
  5. Path query parsing (line 215)
  6. Client readiness check (line 217)
  7. Unary call execution (line 229)

This ensures consistent error types and reporting throughout the entire operation flow.

src/typegate/engine/src/error.rs (5)

6-12: Well-structured error type implementation.

The OpErr struct is cleanly implemented as a wrapper around anyhow::Error with a proper From trait implementation, allowing for seamless conversion from anyhow::Error.


13-20: Good utility methods for error handling.

The implementation provides useful utility methods like get_error_class and map() to facilitate error handling across the codebase. The map() function is particularly useful for easily converting various error types into the OpErr type.


21-30: Standard error trait implementations.

Properly implementing std::error::Error and std::fmt::Display follows Rust best practices for error types, ensuring compatibility with the standard error handling mechanisms.


31-49: Good Deno integration with JsErrorClass.

The implementation of deno_error::JsErrorClass allows for seamless error propagation between Rust and JavaScript contexts, which is essential for the Deno runtime integration.


50-56: Convenient Deref implementation.

The Deref implementation allows direct access to the underlying anyhow::Error, providing convenience while maintaining the semantic benefits of a distinct error type.

src/typegate/engine/src/runtimes/temporal.rs (6)

35-37: Improved error handling in op_temporal_register.

The function now returns Result<(), OpErr> and properly uses map_err(OpErr::map()) to convert errors from building client options and connecting to the client. This creates a more consistent error handling pattern.

Also applies to: 42-47


56-67: Updated error handling in op_temporal_unregister.

The function now returns Result<(), OpErr> and the error case now returns an Err variant with a properly formatted error message instead of using anyhow::bail!. This is more consistent with the new error handling approach.


87-87: Consistent error handling in op_temporal_workflow_start.

The signature has been updated to return Result<String, OpErr> and the error handling now uses map_err(OpErr::map()) consistently. This aligns with the project-wide error handling improvements.

Also applies to: 96-97, 112-113


143-143: Standardized error handling in op_temporal_workflow_signal.

The function now returns Result<(), OpErr> and employs the same error handling pattern using map_err(OpErr::map()) for consistency across the codebase.

Also applies to: 152-153, 166-167


187-187: Unified error handling in op_temporal_workflow_query.

The function signature has been updated to return Result<Vec<String>, OpErr> and error handling has been standardized using map_err(OpErr::map()). The error case at the end also properly converts to OpErr using .into().

Also applies to: 198-199, 213-214, 223-223


248-248: Consistent error handling in op_temporal_workflow_describe.

The function now returns Result<TemporalWorkflowDescribeOutput, OpErr> and maintains the same error handling pattern with map_err(OpErr::map()).

Also applies to: 262-263

src/typegate/engine/src/runtimes/prisma.rs (10)

52-55: Improved error handling in op_prisma_register_engine.

The function now returns Result<(), OpErr> and properly uses map_err(OpErr::map()) for error handling, aligning with the new error handling approach across the codebase.

Also applies to: 65-65


75-75: Consistent error handling in op_prisma_unregister_engine.

The function signature has been updated to return Result<(), OpErr> and the disconnect operation now properly converts errors using map_err(OpErr::map()).

Also applies to: 85-85


106-106: Standardized error handling in op_prisma_query.

The function now returns Result<String, OpErr> and uses the consistent error handling pattern with map_err(OpErr::map()).

Also applies to: 111-113


129-135: Unified error handling in op_prisma_diff.

The function signature has been updated to return Result<Option<(String, Vec<ParsedDiff>)>, OpErr> and the same error handling pattern is applied to both the datamodel formatting and diff operations.


153-157: Consistent error handling in op_prisma_apply.

The function now returns Result<PrismaApplyResult, OpErr> and consistently applies the error handling pattern to the datamodel formatting, build, and apply operations.

Also applies to: 164-167


184-188: Standardized error handling in op_prisma_deploy.

The function signature has been updated to return Result<PrismaDeployOut, OpErr> and maintains consistent error handling with map_err(OpErr::map()) throughout.

Also applies to: 195-195, 198-198


217-221: Unified error handling in op_prisma_create.

The function now returns Result<PrismaCreateResult, OpErr> and maintains the same error handling pattern across all operations.

Also applies to: 228-228, 231-231


239-239: Consistent error handling in op_prisma_reset.

The function signature has been updated to return Result<bool, OpErr> and applies the same error handling pattern to the build and reset operations.

Also applies to: 246-246, 249-249


260-261: Standardized error handling in op_unpack.

The function now returns Result<(), OpErr> and uses the consistent error conversion with map_err(OpErr::map()).


266-267: Standardized error handling in op_archive.

The function signature has been updated to return Result<Option<String>, OpErr> and maintains the same error handling pattern.

import_map.json (4)

3-24: Updated to current JSR standard library modules.

The import map has been updated to use JSR (Deno's package registry) for standard library modules, which is the recommended approach for Deno 2.x. These updates ensure compatibility with the new Deno version.


25-27: Updated AWS SDK dependencies.

The AWS SDK dependencies have been updated from version 3.700.0 to 3.749.0 and changed from ESM.sh to NPM imports, which should offer better stability and compatibility.


48-83: Added SDK and tooling dependencies.

New imports have been added for local tools, typegraph SDK components, and various runtime implementations. This organization improves modularity and allows for better dependency management.


84-94: Added development and testing dependencies.

Additional dependencies for development and testing purposes have been added, including JSON Schema tools, PostgreSQL client, and test utilities, which should improve the development and testing experience.

src/typegate/engine/src/runtimes/substantial.rs (5)

60-60: Improved error type specification throughout function signatures

All functions now consistently return Result<T, OpErr> instead of just Result<T>, making the error type explicit. This enhances type safety and provides clearer error handling contracts.

Also applies to: 89-89, 122-122, 157-157, 184-184, 213-213, 241-241, 269-269, 290-290, 312-312, 340-340, 375-375, 408-408, 436-436, 465-465, 493-493


70-70: Consistent error mapping for backend initialization

Backend initialization errors are now properly mapped to OpErr using map_err(OpErr::map()), ensuring consistent error handling throughout the codebase.

Also applies to: 99-99, 132-132, 167-167, 193-194, 222-223, 250-251, 278-279, 299-300, 321-322, 349-350, 384-385, 417-418, 445-446, 474-475, 502-503


73-73: Consistent error mapping for backend operations

All backend operations now properly map their errors to the OpErr type, ensuring a uniform error handling approach across different operations. This improves error reporting and makes debugging easier.

Also applies to: 101-104, 134-141, 169-177, 196-198, 225-227, 253-255, 281-283, 302-304, 324-326, 352-359, 387-393, 420-422, 448-450, 477-479, 505-507


169-177: Properly handled nested Result with Option in read_schedule

The error handling in op_sub_store_read_schedule correctly handles the complex case where an operation might return an Option<T> wrapped in a Result. The use of transpose() elegantly handles this nested structure.


308-308: Added #[serde] annotations for consistent serialization

The #[serde] annotations have been added to functions that were missing them, ensuring consistent serialization behavior across all operations.

Also applies to: 371-371, 404-404, 461-461

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (1)
.ghjk/lock.json (1)

728-733: 🛠️ Refactor suggestion

Remove Unnecessary Intermediate Deno Version Entry
The entry for "bciqg5gzspxjw7zvrubuyyqskh2244x6mkoxfwugnrx3clyadlegvidi" with version "v2.0.6" is now obsolete since the PR objective is to update Deno to v2.1.2. Removing this entry will simplify the dependency tree and avoid potential confusion.

Proposed diff:

-        "bciqg5gzspxjw7zvrubuyyqskh2244x6mkoxfwugnrx3clyadlegvidi": {
-          "version": "v2.0.6",
-          "buildDepConfigs": {},
-          "portRef": "[email protected]",
-          "specifiedVersion": true
-        },
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8dfe626 and 2fcb038.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (5)
  • .ghjk/lock.json (10 hunks)
  • Cargo.toml (3 hunks)
  • ghjk.ts (1 hunks)
  • src/typegate/src/runtimes/wit_wire/mod.ts (1 hunks)
  • tools/consts.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • src/typegate/src/runtimes/wit_wire/mod.ts
  • ghjk.ts
  • tools/consts.ts
  • Cargo.toml
⏰ Context from checks skipped due to timeout of 90000ms (5)
  • GitHub Check: lint-compat (macos-13, x86_64-apple-darwin, false)
  • GitHub Check: test-website
  • GitHub Check: test-full
  • GitHub Check: lint-compat (macos-14, aarch64-apple-darwin, false)
  • GitHub Check: pre-commit
🔇 Additional comments (6)
.ghjk/lock.json (6)

734-739: Confirm Deno Update to v2.1.2
The new entry for "bciqe7g5m4v5jkg3ubqhogjjntsduyrwxcirqcp6tc3jmjr5af7ojq6a" correctly sets the version to "v2.1.2", which meets the PR objective.


740-760: Rust Toolchain Upgrade to 1.82.0
The update within the "rust_rustup" configuration (entry "bciqduktp7qkb4fwcygoacnivxdnnau6uctz4vb5ovujunqwajoqh4jq") now sets the version to "1.82.0". This change is consistent with the PR objective for updating Rust.


761-788: Enhanced Rust Configuration Details
The subsequent entry ("bciqanj4cddv3wefsg5arfpx4zt5b4q7lok2eghvw5umfub76kxjs4bi") upgrades Rust to "1.82.0" and revises its configuration by switching the profile from "minimal" to "default" and adding new components (e.g., "rust-src") and targets (e.g., "wasm32-unknown-unknown", "wasm32-wasi"). This change considerably improves the Rust installation setup. Please verify that these modifications are intended for enhancing the build environment.


789-794: Inconsistent Deno Version: v2.1.10 Encountered
The entry "bciqerpqq5txgtwiuqy3hvbtjwffexiyue7c5k4itdbslq3h4pegkzlq" is set to "v2.1.10", which deviates from the expected "v2.1.2". Please confirm whether this higher version is intentional (for instance, if it applies to a different use case) or if it should be standardized to v2.1.2.


796-800: Verify Deno Version Variant: v2.1.6
An entry ("bciqelfljimuhjjtwwwgqry5ho5zmmon3uzur6qem27cmmaegsn5pn4i") appears with version "v2.1.6". This is yet another variant alongside v2.1.2 and v2.1.10. It is recommended to review whether these discrepancies are deliberate or if the versions should be consolidated.


801-806: Deno Version Inconsistency: v2.1.4 Detected
The entry "bciqjoiy67soflr46oqoknxpoy2m2qfr5n7x5kc6ed5ptprrbscmvoxy" sets the version to "v2.1.4". Given the PR objective to update to v2.1.2, please check if this deviating version is required or if it should be adjusted for consistency.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 888fb3e and 8ad3ede.

📒 Files selected for processing (14)
  • Cargo.toml (3 hunks)
  • import_map.json (1 hunks)
  • rust-toolchain.toml (1 hunks)
  • src/metagen/src/client_ts/static/mod.ts (1 hunks)
  • src/metagen/src/fdk_rs/mod.rs (1 hunks)
  • src/typegate/src/runtimes/s3.ts (3 hunks)
  • src/typegate/src/sync/typegraph.ts (2 hunks)
  • src/utils/mod.ts (1 hunks)
  • tests/metagen/metagen_test.ts (2 hunks)
  • tests/runtimes/s3/s3_test.ts (4 hunks)
  • tools/Dockerfile (2 hunks)
  • tools/compose/compose.base.yml (2 hunks)
  • tools/compose/compose.subredis.yml (1 hunks)
  • tools/consts.ts (1 hunks)
✅ Files skipped from review due to trivial changes (3)
  • tools/compose/compose.base.yml
  • src/metagen/src/client_ts/static/mod.ts
  • tools/Dockerfile
🚧 Files skipped from review as they are similar to previous changes (6)
  • src/typegate/src/runtimes/s3.ts
  • rust-toolchain.toml
  • tools/consts.ts
  • tests/metagen/metagen_test.ts
  • import_map.json
  • Cargo.toml
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/typegate/src/sync/typegraph.ts (1)
src/utils/mod.ts (1)
  • createS3ClientWithMD5 (8-58)
🔇 Additional comments (8)
src/metagen/src/fdk_rs/mod.rs (1)

436-436: LGTM! Correct target architecture update for WASI Preview 1.

The change from wasm32-wasi to wasm32-wasip1 aligns with the Rust toolchain update and properly reflects the WASI Preview 1 target naming convention. This ensures compatibility with the updated Rust version.

src/typegate/src/sync/typegraph.ts (2)

16-16: LGTM: Import addition for S3 client utility.

The import of createS3ClientWithMD5 aligns with the codebase standardization effort to use a consistent S3 client configuration across all modules.


35-37: LGTM: S3 client initialization standardization.

The change from direct S3Client instantiation to using createS3ClientWithMD5 utility function ensures consistent S3 client behavior across the codebase, particularly for MD5 checksum handling in DeleteObjects operations.

tests/runtimes/s3/s3_test.ts (4)

11-11: LGTM: Import standardization for test consistency.

The import change ensures that tests use the same S3 client configuration as the production code, improving test reliability and consistency.


20-28: LGTM: S3 client initialization in tests.

Using createS3ClientWithMD5 in tests ensures consistency with production code and validates the MD5 checksum functionality.


40-53: LGTM: Enhanced error handling for bucket cleanup.

The addition of try-catch blocks with logging around bucket cleanup operations improves test robustness and debugging capabilities.


72-77: LGTM: Error handling for bucket initialization.

The error handling wrapper around initBucket() provides better visibility into initialization failures during test execution.

src/utils/mod.ts (1)

8-58:

✅ Verification successful

Verify the middleware step and placement.

The middleware is added at the finalizeRequest step, but given that it needs to modify request headers, ensure this is the correct step in the AWS SDK middleware stack for header modifications.


🏁 Script executed:

#!/bin/bash
# Description: Search for AWS SDK middleware documentation and examples to verify correct step usage

# Search for middleware step usage examples in the codebase
rg -A 10 -B 5 "finalizeRequest|buildRequest|initialize" --type ts

# Look for other middleware implementations for reference
rg -A 15 "middlewareStack\.add" --type ts

Length of output: 3619


🌐 Web query:

AWS SDK for JavaScript v3 middleware stack steps and recommended step for modifying HTTP request headers

💡 Result:

The AWS SDK for JavaScript v3 uses a modular middleware stack approach to handle HTTP requests and responses. The middleware stack operates by executing middleware functions in a series of steps during the request lifecycle.

Middleware Stack Steps in AWS SDK v3

The middleware stack includes several predefined steps that represent different stages in processing an HTTP request. The main steps are:

  1. initialize: Initial setup and input processing.
  2. serialize: Converts the input into an HTTP request.
  3. build: Builds the HTTP request.
  4. finalizeRequest: Last step before the request is sent, useful for modifying headers or other request aspects.
  5. deserialize: Processes the HTTP response and converts it into the output type.

Recommended Step for Modifying HTTP Request Headers

To modify HTTP request headers, the recommended step is finalizeRequest. This step happens after the HTTP request is fully constructed, allowing you to safely add or modify headers before the request is sent out.

Example Usage

Here is an example middleware that modifies HTTP headers in the finalizeRequest step:

const addCustomHeaderMiddleware = (next, context) => async (args) => {
  // Modify headers here
  args.request.headers["X-Custom-Header"] = "MyValue";

  // Pass control to the next middleware in the stack
  return next(args);
};

// Add middleware to the middleware stack
client.middlewareStack.add(addCustomHeaderMiddleware, {
  step: "finalizeRequest",
  name: "addCustomHeaderMiddleware",
  priority: "high", // Optional priority to control order
});

Summary

  • The AWS SDK v3 middleware stack steps are: initialize, serialize, build, finalizeRequest, deserialize.
  • To modify HTTP request headers, use the finalizeRequest step.
  • Add your custom middleware to this step to safely modify or add headers.

Let me know if you want me to provide a detailed example or help with integrating this into your client!


Middleware placement is correct. The AWS SDK v3 documentation confirms that finalizeRequest is the recommended step for modifying or adding HTTP headers before the request is sent. No changes required.

@Yohe-Am Yohe-Am changed the title chore: update deno to v2.1.2 chore: update deno to v2.2.4 Jun 5, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

♻️ Duplicate comments (9)
.github/workflows/release.yml (4)

38-42: 🛠️ Refactor suggestion

Duplicate: Secure ghjk install in check-bump job

As in prior reviews, update the domain to raw.githubusercontent.com and ensure proper quoting of the URL:

-      - run: curl -fsSL "https://raw.github.com/metatypedev/ghjk/$GHJK_VERSION/install.sh" | bash
+      - run: curl -fsSL "https://raw.githubusercontent.com/metatypedev/ghjk/${GHJK_VERSION}/install.sh" | bash

106-110: 🛠️ Refactor suggestion

Duplicate: Secure ghjk install in pub-meta-cli job

Apply the same domain and quoting fix here:

-      - run: curl -fsSL "https://raw.github.com/metatypedev/ghjk/$GHJK_VERSION/install.sh" | bash
+      - run: curl -fsSL "https://raw.githubusercontent.com/metatypedev/ghjk/${GHJK_VERSION}/install.sh" | bash

185-190: 🛠️ Refactor suggestion

Duplicate: Secure ghjk install in pub-typegraph job

Consistently update this block as well:

-      - run: curl -fsSL "https://raw.github.com/metatypedev/ghjk/$GHJK_VERSION/install.sh" | bash
+      - run: curl -fsSL "https://raw.githubusercontent.com/metatypedev/ghjk/${GHJK_VERSION}/install.sh" | bash
🧰 Tools
🪛 YAMLlint (1.37.1)

[error] 185-185: trailing spaces

(trailing-spaces)


324-330: 🛠️ Refactor suggestion

Duplicate: Secure ghjk install in bump job

And here too, switch the domain and quote correctly:

-      - run: curl -fsSL "https://raw.github.com/metatypedev/ghjk/$GHJK_VERSION/install.sh" | bash
+      - run: curl -fsSL "https://raw.githubusercontent.com/metatypedev/ghjk/${GHJK_VERSION}/install.sh" | bash
🧰 Tools
🪛 YAMLlint (1.37.1)

[error] 324-324: trailing spaces

(trailing-spaces)

.github/workflows/tests.yml (4)

91-95: 🛠️ Refactor suggestion

Duplicate: Secure ghjk install in pre-commit job

As before, use raw.githubusercontent.com and quoted URL:

-      - run: curl -fsSL "https://raw.github.com/metatypedev/ghjk/$GHJK_VERSION/install.sh" | bash
+      - run: curl -fsSL "https://raw.githubusercontent.com/metatypedev/ghjk/${GHJK_VERSION}/install.sh" | bash

140-147: 🛠️ Refactor suggestion

Duplicate: Secure ghjk install in lint-compat job

Update this block similarly:

-      - run: curl -fsSL "https://raw.github.com/metatypedev/ghjk/$GHJK_VERSION/install.sh" | bash
+      - run: curl -fsSL "https://raw.githubusercontent.com/metatypedev/ghjk/${GHJK_VERSION}/install.sh" | bash

173-177: 🛠️ Refactor suggestion

Duplicate: Fix ghjk install and path expansion in test-website job

Switch the domain, quote the URL, and replace ~ with $HOME:

-      - run: curl -fsSL "https://raw.github.com/metatypedev/ghjk/$GHJK_VERSION/install.sh" | bash
+      - run: curl -fsSL "https://raw.githubusercontent.com/metatypedev/ghjk/${GHJK_VERSION}/install.sh" | bash

-      - run: echo "~/.local/bin" >> $GITHUB_PATH
+      - run: echo "$HOME/.local/bin" >> $GITHUB_PATH

-      - run: echo "BASH_ENV=~/.local/share/ghjk/env.sh" >> $GITHUB_ENV
+      - run: echo "BASH_ENV=$HOME/.local/share/ghjk/env.sh" >> $GITHUB_ENV

283-291: 🛠️ Refactor suggestion

Duplicate: Fix ghjk install and path expansion in test-full job

And apply the same corrections here:

-      - run: curl -fsSL "https://raw.github.com/metatypedev/ghjk/$GHJK_VERSION/install.sh" | bash
+      - run: curl -fsSL "https://raw.githubusercontent.com/metatypedev/ghjk/${GHJK_VERSION}/install.sh" | bash

-      - run: echo "~/.local/bin" >> $GITHUB_PATH
+      - run: echo "$HOME/.local/bin" >> $GITHUB_PATH

-      - run: echo "BASH_ENV=~/.local/share/ghjk/env.sh" >> $GITHUB_ENV
+      - run: echo "BASH_ENV=$HOME/.local/share/ghjk/env.sh" >> $GITHUB_ENV
🧰 Tools
🪛 YAMLlint (1.37.1)

[error] 283-283: trailing spaces

(trailing-spaces)

.ghjk/lock.json (1)

728-733: Remove redundant intermediate Deno version entry
The lock file still includes the "v2.0.6" entry for deno_ghrel, which appears to be an unnecessary holdover given the bump to later versions.

🧹 Nitpick comments (3)
.ghjk/lock.json (1)

2172-2174: Prefix blackboard Deno version with ‘v’ for consistency
The blackboard entry for bciqejuax… lists "version": "2.2.4". All other Deno versions use the v prefix (e.g., "v2.1.2"). Consider updating to "v2.2.4" for uniformity.

tests/runtimes/prisma/query_builder_test.ts (1)

47-49: Address the FIXME comment about polymorphic batch stage.

The comment indicates that the test includes an unintended polymorphic batch stage from the initial plan. This should be tracked and resolved to ensure the test reflects the intended behavior.

Would you like me to create an issue to track this technical debt?

src/typegate/src/runtimes/prisma/prisma.ts (1)

314-316: Enhance error message for better debugging.

The duplicate check is good, but the error message could be more descriptive.

 if (new Set(stageOrder).size != stageOrder.length) {
-  throw new Error("duplicate top stage ids");
+  throw new Error(`Duplicate stage IDs found in batch query order: ${stageOrder.join(", ")}`);
 }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8ad3ede and 61aa19a.

⛔ Files ignored due to path filters (3)
  • .ghjk/deno.lock is excluded by !**/*.lock
  • Cargo.lock is excluded by !**/*.lock
  • deno.lock is excluded by !**/*.lock
📒 Files selected for processing (35)
  • .ghjk/lock.json (10 hunks)
  • .github/workflows/publish-website.yml (1 hunks)
  • .github/workflows/release.yml (5 hunks)
  • .github/workflows/tests.yml (5 hunks)
  • Cargo.toml (3 hunks)
  • ghjk.ts (1 hunks)
  • import_map.json (1 hunks)
  • src/meta-cli/src/deploy/actors/task/deploy/artifacts.rs (1 hunks)
  • src/meta-cli/src/typegraph/rpc/mod.rs (1 hunks)
  • src/metagen/src/client_ts/static/mod.ts (1 hunks)
  • src/metagen/src/fdk_rs/mod.rs (1 hunks)
  • src/mt_deno/src/lib.rs (9 hunks)
  • src/typegate/engine/src/runtimes/wit_wire.rs (3 hunks)
  • src/typegate/src/engine/planner/policies.ts (1 hunks)
  • src/typegate/src/runtimes/prisma/prisma.ts (4 hunks)
  • src/typegate/src/runtimes/substantial.ts (1 hunks)
  • src/typegate/src/runtimes/substantial/agent.ts (1 hunks)
  • src/typegate/src/runtimes/wit_wire/mod.ts (1 hunks)
  • src/typegate/src/services/auth/protocols/oauth2.ts (0 hunks)
  • src/typegraph/core/src/runtimes/prisma/type_utils.rs (1 hunks)
  • src/typegraph/schema/src/validator/injection.rs (1 hunks)
  • src/typegraph/schema/src/validator/mod.rs (1 hunks)
  • src/typegraph/schema/src/validator/types.rs (5 hunks)
  • src/typegraph/schema/src/visitor.rs (1 hunks)
  • src/typegraph/schema/src/visitor2.rs (1 hunks)
  • src/utils/mod.ts (1 hunks)
  • tests/metagen/metagen_test.ts (2 hunks)
  • tests/runtimes/prisma/prisma_test.ts (1 hunks)
  • tests/runtimes/prisma/query_builder_test.ts (1 hunks)
  • tools/Dockerfile (2 hunks)
  • tools/compose/compose.base.yml (3 hunks)
  • tools/consts.ts (1 hunks)
  • tools/cross.Dockerfile (1 hunks)
  • tools/deps.ts (1 hunks)
  • tools/list-duplicates.ts (2 hunks)
💤 Files with no reviewable changes (1)
  • src/typegate/src/services/auth/protocols/oauth2.ts
✅ Files skipped from review due to trivial changes (11)
  • ghjk.ts
  • src/typegraph/schema/src/validator/mod.rs
  • src/typegraph/core/src/runtimes/prisma/type_utils.rs
  • tools/list-duplicates.ts
  • src/meta-cli/src/deploy/actors/task/deploy/artifacts.rs
  • src/typegraph/schema/src/visitor.rs
  • tools/cross.Dockerfile
  • src/typegate/src/runtimes/substantial/agent.ts
  • tools/Dockerfile
  • tools/consts.ts
  • src/typegraph/schema/src/validator/types.rs
🚧 Files skipped from review as they are similar to previous changes (12)
  • src/metagen/src/fdk_rs/mod.rs
  • src/typegate/src/runtimes/wit_wire/mod.ts
  • src/meta-cli/src/typegraph/rpc/mod.rs
  • tools/compose/compose.base.yml
  • src/typegate/src/runtimes/substantial.ts
  • src/typegate/src/engine/planner/policies.ts
  • tests/metagen/metagen_test.ts
  • src/metagen/src/client_ts/static/mod.ts
  • tools/deps.ts
  • Cargo.toml
  • src/typegate/engine/src/runtimes/wit_wire.rs
  • src/mt_deno/src/lib.rs
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/publish-website.yml

[error] 32-32: trailing spaces

(trailing-spaces)

.github/workflows/release.yml

[error] 37-37: trailing spaces

(trailing-spaces)


[error] 185-185: trailing spaces

(trailing-spaces)


[error] 324-324: trailing spaces

(trailing-spaces)

.github/workflows/tests.yml

[error] 172-172: trailing spaces

(trailing-spaces)


[error] 283-283: trailing spaces

(trailing-spaces)

🪛 Biome (1.9.4)
src/utils/mod.ts

[error] 22-78: This code is unreachable

... because this statement will return from the function beforehand

(lint/correctness/noUnreachable)

⏰ Context from checks skipped due to timeout of 90000ms (6)
  • GitHub Check: lint-compat (macos-14, aarch64-apple-darwin, false)
  • GitHub Check: build-docker (linux/arm64, ubuntu-22.04-arm)
  • GitHub Check: build-docker (linux/amd64, custom-ubuntu-large)
  • GitHub Check: lint-compat (macos-13, x86_64-apple-darwin, false)
  • GitHub Check: test-full
  • GitHub Check: pre-commit
🔇 Additional comments (7)
src/typegraph/schema/src/visitor2.rs (1)

22-22: LGTM! Excellent lifetime simplification.

The change from explicit lifetime 'a to anonymous lifetime '_ follows modern Rust best practices and improves code readability without affecting functionality.

src/typegraph/schema/src/validator/injection.rs (1)

21-21: LGTM! Consistent lifetime annotation improvement.

This change aligns with the broader pattern of lifetime simplifications across the codebase and follows Rust best practices by using anonymous lifetime '_ where the compiler can infer the appropriate lifetime.

.ghjk/lock.json (3)

734-739: Approve addition of Deno v2.1.2
The new [email protected] entry aligns with the PR’s stated objective. The format and structure are correct.


842-845: Verify environment sets include the correct Deno portRef
The ghjkEnvProvInstSet___main (and other sets) add the "bciqejuaxov2wky6no34n5s7n5bxmxytz3h4bqqnfxnfgjegynrz7n6i" install. Confirm this ID points to the intended Deno version after consolidation.


2383-2392: Ensure rust_rustup defaultInst matches configResolutions
The blackboard defaultInst for rust_rustup is now "version": "1.85.0" with default profile and components, but the configResolutions entries target 1.82.0. Please confirm and align these two sections.

tests/runtimes/prisma/prisma_test.ts (1)

200-216: Well-structured test for batch query functionality!

The test effectively validates that multiple mutations can be batched in a single request and that each returns a distinct UUID.

src/typegate/src/runtimes/prisma/prisma.ts (1)

189-193: Good fix for batch result handling!

The change correctly addresses the previous limitation by processing all results in the batch instead of just the first item.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🧹 Nitpick comments (3)
src/typegate/src/typegate/artifacts/shared.ts (1)

174-178: Consider adding error handling for file operations.

The file write operations could fail due to permissions or disk space issues. Consider wrapping in try-catch for better error handling.

-    await Deno.mkdir(dirname(targetFile), { recursive: true });
-    const file = (await Deno.open(targetFile, { write: true, create: true }))
-      .writable;
-    await response.body!.pipeTo(file);
+    try {
+      await Deno.mkdir(dirname(targetFile), { recursive: true });
+      const file = (await Deno.open(targetFile, { write: true, create: true }))
+        .writable;
+      await response.body!.pipeTo(file);
+    } catch (error) {
+      throw new ArtifactError(
+        `Failed to save artifact to local cache: ${error.message}`,
+        import.meta,
+      );
+    }
src/typegate/src/runtimes/s3.ts (1)

199-230: Consider using Promise.allSettled for better error handling in parallel uploads.

The current implementation using Promise.all will reject immediately if any upload fails, potentially leaving some uploads incomplete. Consider using Promise.allSettled to allow all uploads to complete and then handle failures appropriately.

-            // Upload all files in parallel
-            await Promise.all(files.map(async (file, i) => {
+            // Upload all files in parallel
+            const results = await Promise.allSettled(files.map(async (file, i) => {
               const filePath = `${prefix}${file.name}`;
               // ... rest of upload logic ...
             }));
+            
+            // Check for failures
+            const failures = results
+              .map((result, i) => ({ result, file: files[i] }))
+              .filter(({ result }) => result.status === 'rejected');
+            
+            if (failures.length > 0) {
+              const errorMessages = failures
+                .map(({ result, file }) => `${file.name}: ${(result as PromiseRejectedResult).reason}`)
+                .join(', ');
+              throw new Error(`Failed to upload ${failures.length} file(s): ${errorMessages}`);
+            }
.ghjk/lock.json (1)

2350-2389: Inconsistent WASM target naming

In the rust_rustup default instance, targets include "wasm32-wasip1", whereas the config resolutions use "wasm32-wasi". Standardize on the correct target name (wasm32-wasi) to avoid ambiguity.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 61aa19a and 5890344.

⛔ Files ignored due to path filters (1)
  • deno.lock is excluded by !**/*.lock
📒 Files selected for processing (11)
  • .ghjk/lock.json (10 hunks)
  • .vscode/settings.json (2 hunks)
  • examples/typegraphs/metagen/rs/fdk.rs (1 hunks)
  • src/typegate/src/runtimes/s3.ts (5 hunks)
  • src/typegate/src/typegate/artifacts/shared.ts (3 hunks)
  • src/typegraph/specs/codegen/rpc/typescript/client.ts (1 hunks)
  • tests/artifacts/artifacts_test.ts (1 hunks)
  • tests/metagen/typegraphs/identities/rs/fdk.rs (1 hunks)
  • tests/metagen/typegraphs/identities/ts/fdk.ts (1 hunks)
  • tests/runtimes/wasm_wire/rust/fdk.rs (1 hunks)
  • tools/installs.ts (1 hunks)
✅ Files skipped from review due to trivial changes (3)
  • examples/typegraphs/metagen/rs/fdk.rs
  • .vscode/settings.json
  • tests/metagen/typegraphs/identities/rs/fdk.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • tools/installs.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/typegate/src/typegate/artifacts/shared.ts (2)
src/typegate/src/typegate/artifacts/local.ts (1)
  • fetch (89-100)
src/typegate/src/typegate/artifacts/mod.ts (1)
  • ArtifactError (21-25)
⏰ Context from checks skipped due to timeout of 90000ms (7)
  • GitHub Check: build-docker (linux/amd64, custom-ubuntu-large)
  • GitHub Check: lint-compat (macos-14, aarch64-apple-darwin, false)
  • GitHub Check: lint-compat (macos-13, x86_64-apple-darwin, false)
  • GitHub Check: test-full
  • GitHub Check: build-docker (linux/arm64, ubuntu-22.04-arm)
  • GitHub Check: test-website
  • GitHub Check: pre-commit
🔇 Additional comments (7)
tests/artifacts/artifacts_test.ts (1)

218-221: LGTM! Improved error handling with better type safety.

The changes enhance error handling by:

  • Using a more descriptive variable name (err instead of e)
  • Adding a type guard to safely access the error message regardless of the thrown value type
  • Following TypeScript best practices for exception handling

This aligns well with the broader error handling refactoring pattern mentioned across the codebase.

tests/metagen/typegraphs/identities/ts/fdk.ts (1)

828-828: LGTM! Good TypeScript practice improvement.

Adding the override keyword makes the method overriding explicit and provides compile-time safety. This aligns with TypeScript best practices and improves code maintainability.

src/typegate/src/runtimes/s3.ts (1)

160-189: Upload implementation using presigned URLs looks good!

The refactored upload method properly:

  • Generates presigned URLs with appropriate expiry
  • Sets correct Content-Type and Content-Length headers
  • Includes comprehensive error handling with response body
  • Maintains good logging for debugging
tests/runtimes/wasm_wire/rust/fdk.rs (1)

119-119: LGTM! Clean version bump.

The version constant update from "0.5.1-rc.3" to "0.5.1-rc.4" aligns with the broader project version updates mentioned in the PR objectives.

src/typegraph/specs/codegen/rpc/typescript/client.ts (2)

82-84: LGTM! Proper Buffer to Uint8Array migration.

The replacement of Node.js Buffer with Uint8Array is appropriate for Deno 2.x compatibility and maintains the same functionality.


86-99: LGTM! Clean unification of reading logic.

The unified do...while loop approach is cleaner than the previous separate handling for Deno and Node environments. The manual Uint8Array concatenation correctly replaces Buffer.concat, and the termination condition on newline byte 0x0a is appropriate.

.ghjk/lock.json (1)

2515-2521: Align alias mapping for rust_rustup

The alias group now points to the new rust_rustup instance ID (bciqpbny46lpewoqqgsmjcpnjqpaqn62dnf6eiowmpif7ap6gthelm6q). Ensure all downstream references (scripts, policies) are updated to this key to prevent resolution errors.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 10

♻️ Duplicate comments (3)
.github/workflows/publish-website.yml (1)

34-34: Quote the URL and use githubusercontent.com for reliability.

As mentioned in the previous review, the URL should be properly quoted for security and reliability.

-      - run: curl -fsSL https://raw.githubusercontent.com/metatypedev/ghjk/$GHJK_VERSION/install.sh | bash
+      - run: curl -fsSL "https://raw.githubusercontent.com/metatypedev/ghjk/${GHJK_VERSION}/install.sh" | bash
.github/workflows/tests.yml (2)

172-172: Remove trailing whitespace.

Static analysis detected trailing spaces on these lines.

Also applies to: 283-283

🧰 Tools
🪛 YAMLlint (1.37.1)

[error] 172-172: trailing spaces

(trailing-spaces)


175-176: 🛠️ Refactor suggestion

Use $HOME instead of tilde for reliable path expansion.

The tilde (~) expansion may not work reliably in GitHub Actions. Use $HOME for consistency with other jobs.

Apply this fix:

-      - run: echo "~/.local/bin" >> $GITHUB_PATH
-      - run: echo "BASH_ENV=~/.local/share/ghjk/env.sh" >> $GITHUB_ENV
+      - run: echo "$HOME/.local/bin" >> "$GITHUB_PATH"
+      - run: echo "BASH_ENV=$HOME/.local/share/ghjk/env.sh" >> "$GITHUB_ENV"

Also applies to: 288-289

🧹 Nitpick comments (2)
.github/workflows/publish-website.yml (1)

32-32: Remove trailing spaces.

Static analysis detected trailing spaces on this line.

-      
+
🧰 Tools
🪛 YAMLlint (1.37.1)

[error] 32-32: trailing spaces

(trailing-spaces)

.github/workflows/release.yml (1)

37-37: Remove trailing whitespace.

Static analysis detected trailing spaces on these lines.

Apply this fix to remove trailing spaces:

-      
+

Also applies to: 185-185, 324-324

🧰 Tools
🪛 YAMLlint (1.37.1)

[error] 37-37: trailing spaces

(trailing-spaces)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7ddbbea and 7d4b678.

⛔ Files ignored due to path filters (1)
  • deno.lock is excluded by !**/*.lock
📒 Files selected for processing (34)
  • .ghjk/lock.json (11 hunks)
  • .github/workflows/publish-website.yml (1 hunks)
  • .github/workflows/release.yml (5 hunks)
  • .github/workflows/tests.yml (5 hunks)
  • .pre-commit-config.yaml (1 hunks)
  • src/typegate/src/runtimes/python.ts (0 hunks)
  • src/typegate/src/runtimes/wasm_wire.ts (0 hunks)
  • src/typegate/src/typegate/mod.ts (1 hunks)
  • src/typegraph/deno/src/io.ts (2 hunks)
  • src/typegraph/deno/src/metagen.ts (2 hunks)
  • src/typegraph/deno/src/params.ts (1 hunks)
  • src/typegraph/deno/src/providers/aws.ts (1 hunks)
  • src/typegraph/deno/src/providers/temporal.ts (1 hunks)
  • src/typegraph/deno/src/runtimes/graphql.ts (1 hunks)
  • src/typegraph/deno/src/runtimes/random.ts (1 hunks)
  • src/typegraph/deno/src/runtimes/wasm.ts (1 hunks)
  • src/typegraph/deno/src/tg_manage.ts (1 hunks)
  • src/typegraph/deno/src/typegraph.ts (1 hunks)
  • src/typegraph/deno/src/utils/func_utils.ts (1 hunks)
  • src/typegraph/deno/src/utils/injection_utils.ts (2 hunks)
  • src/typegraph/specs/codegen/rpc/typescript/client.ts (4 hunks)
  • src/utils/mod.ts (1 hunks)
  • tests/e2e/cli/dev_test.ts (3 hunks)
  • tests/e2e/self_deploy/self_deploy_bad_cred_test.ts (0 hunks)
  • tests/e2e/self_deploy/self_deploy_bad_cred_test.ts.disabled (1 hunks)
  • tests/e2e/self_deploy/self_deploy_test.ts (0 hunks)
  • tests/e2e/self_deploy/self_deploy_test.ts.disable (1 hunks)
  • tests/metagen/metagen_test.ts (2 hunks)
  • tests/multi_typegraph/multi_typegraph.ts (0 hunks)
  • tests/patterns/worker_manager/pooling_test.ts (2 hunks)
  • tests/runtimes/python/python_sync_test.ts (1 hunks)
  • tests/runtimes/s3/s3_test.ts (2 hunks)
  • tools/installs.ts (1 hunks)
  • tools/list-duplicates.ts (2 hunks)
💤 Files with no reviewable changes (5)
  • src/typegate/src/runtimes/python.ts
  • tests/multi_typegraph/multi_typegraph.ts
  • src/typegate/src/runtimes/wasm_wire.ts
  • tests/e2e/self_deploy/self_deploy_bad_cred_test.ts
  • tests/e2e/self_deploy/self_deploy_test.ts
✅ Files skipped from review due to trivial changes (15)
  • src/typegraph/deno/src/utils/func_utils.ts
  • tests/runtimes/python/python_sync_test.ts
  • src/typegraph/deno/src/params.ts
  • src/typegraph/deno/src/typegraph.ts
  • src/typegraph/deno/src/runtimes/wasm.ts
  • src/typegraph/deno/src/providers/aws.ts
  • src/typegraph/deno/src/runtimes/random.ts
  • src/typegraph/deno/src/providers/temporal.ts
  • tests/patterns/worker_manager/pooling_test.ts
  • src/typegraph/deno/src/utils/injection_utils.ts
  • src/typegraph/deno/src/metagen.ts
  • tests/runtimes/s3/s3_test.ts
  • tools/list-duplicates.ts
  • src/typegraph/deno/src/io.ts
  • tests/e2e/cli/dev_test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
  • tools/installs.ts
  • src/typegate/src/typegate/mod.ts
  • tests/metagen/metagen_test.ts
  • src/utils/mod.ts
  • src/typegraph/specs/codegen/rpc/typescript/client.ts
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/publish-website.yml

[error] 32-32: trailing spaces

(trailing-spaces)

.github/workflows/release.yml

[error] 37-37: trailing spaces

(trailing-spaces)


[error] 185-185: trailing spaces

(trailing-spaces)


[error] 324-324: trailing spaces

(trailing-spaces)

.github/workflows/tests.yml

[error] 172-172: trailing spaces

(trailing-spaces)


[error] 283-283: trailing spaces

(trailing-spaces)

🔇 Additional comments (14)
src/typegraph/deno/src/runtimes/graphql.ts (1)

4-4: Excellent optimization using type-only import.

Converting the Effect import to a type-only import is a best practice since it's only used for type annotations. This improves build performance and reduces bundle size.

src/typegraph/deno/src/tg_manage.ts (1)

27-27: Good simplification removing unnecessary async.

The method doesn't perform any asynchronous operations, so removing the async keyword is correct and improves the API by making it synchronous.

.pre-commit-config.yaml (2)

44-50: Verify the impact of disabling file type filtering.

Commenting out the types configuration for deno-lint means it will now run on all files instead of just JavaScript/TypeScript files. This could significantly slow down pre-commit hooks and may cause unnecessary processing.

Please confirm this change is intentional and consider the performance impact. If broader coverage is needed, consider using more specific files patterns instead.


56-58: Verify the impact of disabling file type filtering for eslint.

Similar to the deno-lint hook, removing types_or filtering means eslint will run on all files in the docs/metatype.dev directory, not just TypeScript files.

Consider whether this broader scope is necessary or if it might cause performance issues.

.github/workflows/publish-website.yml (1)

32-37: Manual ghjk setup correctly replaces the action.

The manual installation steps properly recreate the functionality of the removed metatypedev/setup-ghjk action with correct PATH and environment variable setup.

🧰 Tools
🪛 YAMLlint (1.37.1)

[error] 32-32: trailing spaces

(trailing-spaces)

.github/workflows/release.yml (2)

38-42: LGTM! Manual ghjk installation is properly implemented.

The replacement of the setup-ghjk action with manual installation steps is correctly implemented with proper quoting and the correct GitHub domain.

Also applies to: 106-109, 186-190, 326-329


61-61: Prerelease condition updated to use 'rc' instead of 'alpha'.

The change from checking for 'alpha' to 'rc' in the prerelease condition appears intentional for the release workflow.

tests/e2e/self_deploy/self_deploy_test.ts.disable (1)

1-45: Well-structured disabled test awaiting feature completion.

The test implementation looks solid with proper setup, assertions, and cleanup. The FIXME comment appropriately indicates this is disabled pending MET-754 implementation.

tests/e2e/self_deploy/self_deploy_bad_cred_test.ts.disabled (1)

1-41: Good negative test case for authentication failure.

The test properly implements negative testing with invalid credentials and correctly verifies the expected 401 Unauthorized error. The use of unreachable() ensures the test fails if deployment unexpectedly succeeds.

.github/workflows/tests.yml (2)

91-95: LGTM! Manual ghjk installation correctly implemented.

The replacement of the setup-ghjk action with manual installation steps is properly implemented with correct quoting and domain.

Also applies to: 144-147


295-295: Shell tracing enabled for debugging.

The addition of set -x will help with debugging the test-full job execution.

.ghjk/lock.json (3)

819-823: Confirm Deno v2.2.4 entry
This block correctly adds the target Deno version v2.2.4. Verify that the blackboard moduleSpecifier and all allowedBuildDeps include the new port ID so builds succeed.


825-847: Approve Rust 1.85.0 config resolution
This block updates rust_rustup to 1.85.0 with the default profile and rust-src component. It aligns with the PR objectives and no further changes are needed here.


2385-2424: Approve defaultInst Rust version bump
The defaultInst for the rust_rustup port is now set to 1.85.0 with the correct profile. This aligns with the toolchain upgrade—no further action required here.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7d4b678 and 256b6f7.

📒 Files selected for processing (89)
  • .github/workflows/tests.yml (6 hunks)
  • deno.jsonc (4 hunks)
  • src/metagen/fixtures/client_ts/main.ts (1 hunks)
  • src/typegate/src/config/types.ts (1 hunks)
  • src/typegate/src/engine/planner/args.ts (2 hunks)
  • src/typegate/src/engine/planner/mod.ts (1 hunks)
  • src/typegate/src/runtimes/deno/deno.ts (1 hunks)
  • src/typegate/src/runtimes/deno/hooks/mod.ts (4 hunks)
  • src/typegate/src/runtimes/deno/types.ts (1 hunks)
  • src/typegate/src/runtimes/deno/worker.ts (1 hunks)
  • src/typegate/src/runtimes/deno/worker_manager.ts (1 hunks)
  • src/typegate/src/runtimes/grpc.ts (1 hunks)
  • src/typegate/src/runtimes/patterns/worker_manager/deno.ts (2 hunks)
  • src/typegate/src/runtimes/patterns/worker_manager/mod.ts (1 hunks)
  • src/typegate/src/runtimes/patterns/worker_manager/pooling.ts (1 hunks)
  • src/typegate/src/runtimes/patterns/worker_manager/wasm.ts (1 hunks)
  • src/typegate/src/runtimes/python.ts (1 hunks)
  • src/typegate/src/runtimes/python/hooks/mod.ts (2 hunks)
  • src/typegate/src/runtimes/random.ts (1 hunks)
  • src/typegate/src/runtimes/s3.ts (5 hunks)
  • src/typegate/src/runtimes/substantial.ts (1 hunks)
  • src/typegate/src/runtimes/substantial/agent.ts (1 hunks)
  • src/typegate/src/runtimes/substantial/deno_context.ts (1 hunks)
  • src/typegate/src/runtimes/substantial/filter_utils.ts (1 hunks)
  • src/typegate/src/runtimes/substantial/types.ts (1 hunks)
  • src/typegate/src/runtimes/substantial/worker.ts (1 hunks)
  • src/typegate/src/runtimes/substantial/workflow_worker_manager.ts (1 hunks)
  • src/typegate/src/runtimes/typegraph.ts (1 hunks)
  • src/typegate/src/runtimes/typegraph/helpers.ts (1 hunks)
  • src/typegate/src/runtimes/typegraph/type_emitter.ts (1 hunks)
  • src/typegate/src/runtimes/typegraph/visibility.ts (1 hunks)
  • src/typegate/src/runtimes/utils/deno.ts (1 hunks)
  • src/typegate/src/runtimes/wasm/worker.ts (1 hunks)
  • src/typegate/src/runtimes/wasm/worker_manager.ts (1 hunks)
  • src/typegate/src/runtimes/wasm_reflected.ts (1 hunks)
  • src/typegate/src/runtimes/wasm_wire.ts (1 hunks)
  • src/typegate/src/sync/typegraph.ts (2 hunks)
  • src/typegate/src/transports/graphql/typegraph.ts (1 hunks)
  • src/typegate/src/typegate/hooks.ts (2 hunks)
  • src/typegate/src/typegate/mod.ts (2 hunks)
  • src/typegate/src/typegate/rate_limiter.ts (1 hunks)
  • src/typegate/src/typegraph/mod.ts (1 hunks)
  • src/typegate/src/typegraph/utils.ts (1 hunks)
  • src/typegate/src/types.ts (1 hunks)
  • src/typegate/src/utils/mod.ts (1 hunks)
  • tests/artifacts/artifacts_test.ts (2 hunks)
  • tests/auth/auth_test.ts (1 hunks)
  • tests/dedup/dedup_test.ts (1 hunks)
  • tests/e2e/cli/dev_test.ts (3 hunks)
  • tests/e2e/cli/watch_test.ts (1 hunks)
  • tests/e2e/published/common.ts (1 hunks)
  • tests/e2e/typegraph/typegraph_test.ts (1 hunks)
  • tests/e2e/website/website_test.ts (1 hunks)
  • tests/metagen/metagen_test.ts (3 hunks)
  • tests/metagen/typegraphs/sample/ts/main.ts (1 hunks)
  • tests/policies/policies_test.ts (1 hunks)
  • tests/rate_limiter/rate_limiter_test.ts (1 hunks)
  • tests/runtimes/graphql/graphql_test.ts (1 hunks)
  • tests/runtimes/grpc/grpc_test.ts (1 hunks)
  • tests/runtimes/kv/kv_test.ts (1 hunks)
  • tests/runtimes/prisma/one_to_one_test.ts (1 hunks)
  • tests/runtimes/prisma/query_builder_test.ts (1 hunks)
  • tests/runtimes/prisma/schema_generation_test.ts (1 hunks)
  • tests/runtimes/prisma/type_duplication_test.ts (1 hunks)
  • tests/runtimes/python/python_sync_test.ts (1 hunks)
  • tests/runtimes/python/python_test.ts (1 hunks)
  • tests/runtimes/substantial/common.ts (1 hunks)
  • tests/runtimes/substantial/filter_utils_test.ts (1 hunks)
  • tests/runtimes/substantial/workflows/child_workflow.ts (1 hunks)
  • tests/runtimes/substantial/workflows/workflow.ts (1 hunks)
  • tests/runtimes/temporal/temporal_test.ts (1 hunks)
  • tests/runtimes/wasm_wire/wasm_wire_test.ts (1 hunks)
  • tests/sync/sync_config_test.ts (1 hunks)
  • tests/sync/sync_force_remove_test.ts (2 hunks)
  • tests/type_nodes/union_test.ts (1 hunks)
  • tests/utils/hooks.ts (1 hunks)
  • tests/utils/meta.ts (1 hunks)
  • tests/utils/migrations.ts (1 hunks)
  • tests/utils/mock_fetch.ts (1 hunks)
  • tests/utils/mod.ts (1 hunks)
  • tests/utils/query/file_extractor.ts (1 hunks)
  • tests/utils/query/graphql_query.ts (1 hunks)
  • tests/utils/query/mod.ts (1 hunks)
  • tests/utils/query/rest_query.ts (1 hunks)
  • tests/utils/single_register.ts (1 hunks)
  • tests/utils/test.ts (2 hunks)
  • tests/utils/test_module.ts (1 hunks)
  • tools/list-duplicates.ts (2 hunks)
  • tools/tree-view-web.ts (1 hunks)
✅ Files skipped from review due to trivial changes (52)
  • tests/e2e/typegraph/typegraph_test.ts
  • tests/runtimes/substantial/workflows/workflow.ts
  • src/typegate/src/runtimes/wasm_reflected.ts
  • tests/runtimes/wasm_wire/wasm_wire_test.ts
  • tests/runtimes/substantial/workflows/child_workflow.ts
  • tests/runtimes/grpc/grpc_test.ts
  • src/typegate/src/runtimes/utils/deno.ts
  • src/typegate/src/config/types.ts
  • src/typegate/src/runtimes/substantial/deno_context.ts
  • src/typegate/src/runtimes/wasm_wire.ts
  • src/typegate/src/runtimes/deno/types.ts
  • src/typegate/src/runtimes/substantial/types.ts
  • tests/rate_limiter/rate_limiter_test.ts
  • src/metagen/fixtures/client_ts/main.ts
  • src/typegate/src/runtimes/patterns/worker_manager/wasm.ts
  • src/typegate/src/typegraph/utils.ts
  • tests/runtimes/prisma/query_builder_test.ts
  • src/typegate/src/runtimes/wasm/worker.ts
  • src/typegate/src/runtimes/deno/deno.ts
  • tests/utils/query/mod.ts
  • tests/runtimes/kv/kv_test.ts
  • tests/auth/auth_test.ts
  • src/typegate/src/types.ts
  • tests/metagen/typegraphs/sample/ts/main.ts
  • src/typegate/src/runtimes/substantial/workflow_worker_manager.ts
  • tests/runtimes/prisma/type_duplication_test.ts
  • tests/sync/sync_config_test.ts
  • src/typegate/src/runtimes/python.ts
  • tests/utils/query/rest_query.ts
  • tests/utils/meta.ts
  • tests/utils/migrations.ts
  • src/typegate/src/runtimes/substantial/filter_utils.ts
  • tests/runtimes/prisma/schema_generation_test.ts
  • tests/e2e/cli/watch_test.ts
  • tests/dedup/dedup_test.ts
  • src/typegate/src/runtimes/wasm/worker_manager.ts
  • tests/utils/mod.ts
  • tests/runtimes/python/python_test.ts
  • tests/runtimes/python/python_sync_test.ts
  • src/typegate/src/typegate/rate_limiter.ts
  • tests/e2e/published/common.ts
  • src/typegate/src/runtimes/patterns/worker_manager/mod.ts
  • src/typegate/src/runtimes/substantial/agent.ts
  • src/typegate/src/runtimes/patterns/worker_manager/pooling.ts
  • src/typegate/src/runtimes/typegraph.ts
  • src/typegate/src/runtimes/typegraph/type_emitter.ts
  • tests/utils/query/graphql_query.ts
  • tests/policies/policies_test.ts
  • src/typegate/src/runtimes/typegraph/visibility.ts
  • tests/utils/single_register.ts
  • src/typegate/src/transports/graphql/typegraph.ts
  • tests/runtimes/prisma/one_to_one_test.ts
🚧 Files skipped from review as they are similar to previous changes (17)
  • src/typegate/src/runtimes/patterns/worker_manager/deno.ts
  • src/typegate/src/runtimes/python/hooks/mod.ts
  • tests/artifacts/artifacts_test.ts
  • tools/list-duplicates.ts
  • src/typegate/src/engine/planner/args.ts
  • deno.jsonc
  • tests/metagen/metagen_test.ts
  • src/typegate/src/sync/typegraph.ts
  • src/typegate/src/typegate/mod.ts
  • tests/sync/sync_force_remove_test.ts
  • src/typegate/src/typegate/hooks.ts
  • src/typegate/src/runtimes/deno/hooks/mod.ts
  • .github/workflows/tests.yml
  • tests/utils/test.ts
  • tests/e2e/cli/dev_test.ts
  • src/typegate/src/runtimes/s3.ts
  • src/typegate/src/runtimes/substantial.ts
⏰ Context from checks skipped due to timeout of 90000ms (5)
  • GitHub Check: test-full
  • GitHub Check: build-docker (linux/arm64, ubuntu-22.04-arm)
  • GitHub Check: build-docker (linux/amd64, custom-ubuntu-large)
  • GitHub Check: test-website
  • GitHub Check: pre-commit
🔇 Additional comments (23)
tests/type_nodes/union_test.ts (1)

4-4: LGTM! Good use of type-only import.

Converting to a type-only import is appropriate here since JSONValue is only used for type annotations throughout the file. This improves clarity and aligns with TypeScript best practices.

tests/runtimes/temporal/temporal_test.ts (1)

6-6: LGTM! Good use of type-only import.

Converting to a type-only import is appropriate since MetaTest is only used for type annotations. This improves bundling efficiency and makes the code intent clearer.

tests/runtimes/graphql/graphql_test.ts (1)

4-4: LGTM! Good practice using type-only imports.

The conversion to a type-only import is appropriate since QueryEngine is only used for type annotations (line 16) and not as a runtime value. This improves bundle efficiency and clarifies the intent of the import.

tests/utils/mock_fetch.ts (1)

11-11: LGTM! Clean TypeScript type-only import improvement.

The use of type modifiers for MatchHandler and Routes correctly identifies these as type-only imports, improving bundling efficiency and type clarity. The router function is appropriately left without the modifier since it's used at runtime.

src/typegate/src/runtimes/substantial/worker.ts (1)

7-7: LGTM! Proper use of type-only imports.

The conversion to type-only imports for WorkflowEvent and WorkflowMessage is correct since these are only used as types throughout the file (in type annotations and satisfies clauses). This improves code clarity and bundling efficiency while maintaining Interrupt as a runtime import since it's used for Interrupt.getTypeOf().

src/typegate/src/engine/planner/mod.ts (3)

21-21: Excellent use of type-only import for better code clarity.

The type modifier for StageMetadata correctly indicates this import is used solely for type annotations, improving code clarity and bundling efficiency.


26-26: Good modernization with type-only import syntax.

Converting to import type for Runtime follows TypeScript best practices and clearly indicates this is used only for type annotations.


28-28: Proper type-only import implementation.

The import type syntax for Injection correctly reflects its usage as a type annotation only, aligning with the codebase modernization effort.

tests/runtimes/substantial/filter_utils_test.ts (1)

8-9: Excellent use of type-only imports!

Converting ExecutionStatus and Expr to type-only imports is the correct approach since they're used exclusively for type annotations throughout the file. This improves clarity and can reduce bundle size.

tests/runtimes/substantial/common.ts (1)

7-8: Well-executed type-only import conversion!

Converting MetaTestCleanupFn and Expr to type-only imports is appropriate since they're used exclusively for type annotations (function parameters and type constraints). This change aligns with TypeScript best practices and the broader pattern mentioned in the PR.

src/typegate/src/runtimes/typegraph/helpers.ts (1)

5-6: LGTM! Proper use of type-only imports.

The conversion to import type is appropriate since TypeGraphDS, ObjectNode, and PolicyIndices are only used for type annotations in function parameters and return types.

tools/tree-view-web.ts (1)

11-11: LGTM! Correct type-only import usage.

The conversion to import type is appropriate since TypeGraphDS is only used for the array type annotation on line 42.

src/typegate/src/typegraph/mod.ts (1)

42-42: LGTM! Proper mixed import pattern.

The change correctly uses type keyword for GeneratorNode (used only for type annotations) while keeping randomizeRecursively as a runtime import (used as a function call on line 334).

src/typegate/src/runtimes/random.ts (2)

5-5: LGTM! Appropriate type-only import.

The conversion to import type is correct since ComputeStage is only used for type annotations in function parameters and return types.


11-11: LGTM! Appropriate type-only import.

The conversion to import type is correct since TypeGraphDS is only used for the class property type annotation on line 21.

tests/utils/test_module.ts (1)

5-5: Excellent use of type-only imports!

This change correctly converts ShellOptions and ShellOutput to type-only imports while keeping shell as a runtime import. This improves bundling efficiency and makes the code more explicit about compile-time vs runtime dependencies.

tests/utils/query/file_extractor.ts (1)

4-4: Proper type-only import conversion.

The Variables import is correctly converted to type-only since it's used solely for the parameter type annotation in extractFilesFromVars. This follows TypeScript best practices and improves bundling efficiency.

tests/e2e/website/website_test.ts (2)

7-7: Correct type-only import for MetaTest.

The MetaTest import is properly converted to type-only since it's used exclusively for type annotations in function parameters and the prepare record type definition.


9-9: Correct type-only import for TypeGraphDS.

The TypeGraphDS import is appropriately converted to type-only since it's used only for type annotations in array type casting and function parameters. This improves bundling efficiency and code clarity.

src/typegate/src/runtimes/grpc.ts (1)

7-7: LGTM! Correct implementation of type-only imports.

The conversion to import type for Logger, Resolver, and RuntimeInitParams is appropriate since these are used solely for type annotations. This improves compilation efficiency and code clarity.

Also applies to: 9-9

src/typegate/src/runtimes/deno/worker.ts (1)

5-5: LGTM! Proper type-only import usage.

Converting DenoEvent and DenoMessage to type-only imports is correct since they're used solely for type annotations (MessageEvent<DenoMessage> and satisfies DenoEvent).

src/typegate/src/runtimes/deno/worker_manager.ts (1)

12-15: LGTM! Excellent distinction between runtime and type-only imports.

The import changes correctly categorize each import:

  • TaskId, TaskContext, DenoEvent, DenoMessage, TaskSpec → type-only (used solely for type annotations)
  • hostcall → runtime import (used as function call)
  • HostCallCtx → type-only (used for parameter typing)

This mixed import pattern on line 13 is a great example of proper TypeScript import semantics.

tests/utils/hooks.ts (1)

4-4: LGTM! Type-only imports correctly applied.

The conversion to type-only imports for SyncConfig and S3ClientConfig is appropriate since these are only used as type annotations. S3Client correctly remains a regular import as it's used for runtime instantiation.

Also applies to: 7-7

Copy link

codecov bot commented Jun 8, 2025

Codecov Report

Attention: Patch coverage is 70.00000% with 60 lines in your changes missing coverage. Please review.

Project coverage is 80.57%. Comparing base (6f6851c) to head (256b6f7).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/typegate/engine/bindings.ts 9.09% 20 Missing ⚠️
src/typegate/src/runtimes/s3.ts 84.44% 7 Missing ⚠️
src/typegate/src/services/rest_service.ts 45.45% 5 Missing and 1 partial ⚠️
src/typegate/src/typegate/mod.ts 60.00% 6 Missing ⚠️
src/typegate/src/services/artifact_service.ts 0.00% 5 Missing ⚠️
src/typegate/src/services/graphql_service.ts 76.47% 4 Missing ⚠️
src/typegate/src/typegate/artifacts/shared.ts 69.23% 4 Missing ⚠️
src/typegate/src/services/auth/protocols/jwt.ts 50.00% 2 Missing ⚠️
src/typegate/src/runtimes/deno/hooks/mod.ts 93.75% 1 Missing ⚠️
...pegate/src/runtimes/prisma/hooks/run_migrations.ts 0.00% 1 Missing ⚠️
... and 4 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #923      +/-   ##
==========================================
- Coverage   80.99%   80.57%   -0.43%     
==========================================
  Files         143      145       +2     
  Lines       18046    17341     -705     
  Branches     1967     1968       +1     
==========================================
- Hits        14616    13972     -644     
+ Misses       3412     3351      -61     
  Partials       18       18              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Yohe-Am Yohe-Am merged commit 715ffc9 into main Jun 8, 2025
21 of 29 checks passed
@Yohe-Am Yohe-Am deleted the chore/deno-2 branch June 8, 2025 20:48
@coderabbitai coderabbitai bot mentioned this pull request Jun 16, 2025
3 tasks
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.

5 participants