use sticky disk grpc port from the env if possible

[aayushshah15]

Important

Add GitHub Action for managing sticky disks, including workflows, core logic, and documentation updates.

• Workflows:
  • Add basic.yaml for testing sticky disks with multiple iterations.
  • Add build.yaml to set up Node.js, Buf, and check for uncommitted changes post-build.
  • Add bump-tag.yaml to automate version tag updates.
• Core Functionality:
  • Implement sticky disk management in main.ts and post.ts for mounting, formatting, and committing disks.
  • Use utils.ts to create a gRPC client for sticky disk operations.
• Documentation:
  • Update README.md to describe sticky disk usage, architecture, and performance benefits.
• Configuration:
  • Update action.yml to define inputs for sticky disk key and path.
  • Update package.json to include necessary dependencies and scripts for building and testing.

^{This description was created by for 617cc46. It will automatically update as commits are pushed.}

[ellipsis-dev]

❌ Changes requested. Reviewed everything up to 617cc46 in 2 minutes and 33 seconds

More details

• Looked at 978 lines of code in 10 files
• Skipped 9 files when reviewing.
• Skipped posting 14 drafted comments based on config settings.

1. .github/workflows/basic.yaml:15

• Draft comment:
  The directory creation step uses 'sudo mkdir -p' but the description is incomplete. Consider renaming the step (e.g., 'Create mount directory') for clarity.
• Reason this comment was not posted:
  Confidence changes required: 30% <= threshold 50%
  None

2. .github/workflows/build.yaml:38

• Draft comment:
  The build step checks for uncommitted changes. This is good, but consider adding explicit logging of unexpected file changes for more clarity.
• Reason this comment was not posted:
  Confidence changes required: 20% <= threshold 50%
  None

3. .github/workflows/bump-tag.yaml:21

• Draft comment:
  The bump tag workflow is simple and clear. No changes required.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

4. README.md:28

• Draft comment:
  The README provides a nice architectural overview with images. Consider adding more technical specifics if space permits.
• Reason this comment was not posted:
  Confidence changes required: 10% <= threshold 50%
  None

5. action.yml:8

• Draft comment:
  Inputs 'key' and 'path' are clearly defined. Ensure that required field validation happens in code as well.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

6. package.json:6

• Draft comment:
  The build script now builds both main and post modules. Consider ensuring that the output directories are correctly cleaned before build.
• Reason this comment was not posted:
  Confidence changes required: 20% <= threshold 50%
  None

7. src/main.ts:116

• Draft comment:
  The destructuring assignment within the try (line 116) could be split for readability. Consider assigning the result of mountStickyDisk to a variable before destructuring.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50%
  None

8. src/main.ts:67

• Draft comment:
  Formatting command for the block device uses sudo mkfs.ext4 with several options. Ensure these options are proper for all target environments and consider handling any potential formatting errors more gracefully.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50%
  None

9. src/main.ts:90

• Draft comment:
  After mounting the disk (line 90), ownership is changed using chown with shell expansion. Ensure that the environment executing the command supports this syntax.
• Reason this comment was not posted:
  Confidence changes required: 30% <= threshold 50%
  None

10. src/post.ts:118

• Draft comment:
  The unmount process retries up to 10 times with fixed delay; consider making retry attempts and delay configurable.
• Reason this comment was not posted:
  Confidence changes required: 40% <= threshold 50%
  None

11. src/utils.ts:6

• Draft comment:
  The GRPC port is read from an environment variable with a fallback to 5557. Ensure this default is appropriate for all deployment contexts.
• Reason this comment was not posted:
  Confidence changes required: 30% <= threshold 50%
  None

12. .github/workflows/basic.yaml:15

• Draft comment:
  Step name 'Create directory called' is vague. Consider specifying the directory name (e.g. 'Create directory ./shouldseethis') for clarity.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
  While the suggested name is more precise, this is a very minor stylistic issue. The current name doesn't cause any confusion about what the step does since the command is right there. The workflow file is a test file and the step names are clear enough for debugging purposes. This feels like unnecessary nitpicking.
  The clearer step name could make it easier to debug workflow failures at a glance without having to look at the actual commands.
  However, the step is simple, its purpose is obvious from the command, and this is a test workflow where absolute precision in step names isn't critical.
  This comment should be deleted as it's too minor and doesn't meaningfully improve code quality or prevent potential issues.

13. src/post.ts:98

• Draft comment:
  Validate and sanitize 'stickyDiskPath' before using it in shell commands (e.g. in mount | grep ${stickyDiskPath}) to mitigate potential command injection risks.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
  Command injection is a serious security issue. However, STICKYDISK_PATH comes from GitHub Actions state (getState), which is internal to the action and not user input. The state is set by the action itself in a pre step, not by external users. This makes command injection extremely unlikely since the path value is controlled by the action.
  I could be wrong about the security of GitHub Actions state - maybe there are ways for workflows to tamper with it. Also, even internal values should sometimes be sanitized as defense in depth.
  While defense in depth is good, GitHub Actions state is designed to be secure between steps. Over-sanitizing internal values adds complexity for minimal security benefit.
  The comment raises a theoretical security concern but the risk is very low since the value comes from GitHub Actions internal state. The comment should be removed.

14. README.md:67

• Draft comment:
  Typographical error: 'zero-confg' should be corrected to 'zero-config'.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
  While this is technically correct - there is a typo - we need to consider if this rises to the level of importance that warrants a PR comment. It's a very minor typo in documentation that doesn't impact functionality or understanding. The rules state we should not make obvious or unimportant comments. Typos in documentation generally fall into this category unless they significantly impact meaning.
  The typo could potentially cause confusion for users searching for "zero-config" functionality. Documentation quality is important for user experience.
  While documentation quality matters, this particular typo is minor enough that most readers would understand the intended meaning, and it doesn't warrant the overhead of a PR comment.
  This comment should be deleted as it points out a very minor documentation issue that doesn't significantly impact understanding or functionality.

Workflow ID: wflow_BU6h56uTLMhwizuv

---

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

Typo detected: 'zero-confg' should be 'zero-config'.

Suggested change

Bazel's remote cache can significantly improve build times, but uploading and downloading cached artifacts can still be a bottleneck. Using sticky disks with Blacksmith runners provides near-instant access to your Bazel caches as they are bind mounted into your runners on demand. Our [`useblacksmith/setup-bazel@v2`](https://github.com/useblacksmith/setup-bazel) action is a zero-confg way to use sticky disks to store the disk, repository, and external cache.

Bazel's remote cache can significantly improve build times, but uploading and downloading cached artifacts can still be a bottleneck. Using sticky disks with Blacksmith runners provides near-instant access to your Bazel caches as they are bind mounted into your runners on demand. Our [`useblacksmith/setup-bazel@v2`](https://github.com/useblacksmith/setup-bazel) action is a zero-config way to use sticky disks to store the disk, repository, and external cache.

[ellipsis-dev]

Duplicate import from @actions/core; consider consolidating the individual imports (getInput, saveState) with the import of * as core.

Suggested change

import { getInput, saveState } from "@actions/core";

[ellipsis-dev]

Ensure the timeout is always cleared by moving clearTimeout(timeoutId) into a finally block, so that if getStickyDisk throws, the timer is cleared.

[ellipsis-dev]

Sanitize inputs (e.g. stickyDiskPath) used in shell commands to avoid potential command injection risks.

Suggested change

	await execAsync(`mkdir -p ${stickyDiskPath}`);
	await execAsync(`mkdir -p ${path.resolve(stickyDiskPath)}`);

[ellipsis-dev]

Consider making the baseUrl configurable instead of hardcoding 'http://192.168.127.1', which would improve flexibility in different environments.

[ellipsis-dev]

Typographical inconsistency noticed: The function name commitStickydisk (line 9) uses a lowercase 'd' in 'disk', whereas other references (e.g., in variable names and comments) use StickyDisk. For clarity and consistency, consider renaming the function to commitStickyDisk.