Skip to content

Staging#7

Merged
zkbkb merged 31 commits intomainfrom
staging
Jul 26, 2025
Merged

Staging#7
zkbkb merged 31 commits intomainfrom
staging

Conversation

@zkbkb
Copy link
Copy Markdown
Owner

@zkbkb zkbkb commented Jul 26, 2025

PR Type

Enhancement, Tests


Description

  • Add versioning support and build-time version injection

  • Create comprehensive CI/CD workflows for testing and releases

  • Implement test release script for local verification

  • Add ESLint configuration and linting support


Diagram Walkthrough

flowchart LR
  A["Version Support"] --> B["CI/CD Workflows"]
  B --> C["Test Release Script"]
  C --> D["Linting & Quality"]
  E["main.go"] --> A
  F[".github/workflows/"] --> B
  G["test-release.sh"] --> C
  H[".eslintrc.json"] --> D
Loading

File Walkthrough

Relevant files

zkbkb and others added 30 commits July 26, 2025 08:07
…port

- Added new dependencies in go.mod
- Updated README with new badges for version and release status
- Introduced a version variable in main.go for build-time versioning
- Created a test-release.sh script for verifying builds and dependencies
- Enhanced GitHub Actions workflow to include verification steps for Node.js and Go before release
- Introduced a new test_release.yml workflow for verifying builds and testing releases.
- Introduced a new GitHub Actions workflow for testing builds, including Node.js and Go verification, linting, and security scanning.
- Changed the release badge in README to point to the production release workflow.
Fix (`test_build.yml`) to automatically validate code and build on every push and pull request.

The new workflow performs the following checks:
- Builds and tests both the Go and Node.js applications across multiple platforms (Linux, macOS, Windows).
- Lints Go and JavaScript code to ensure style consistency.
- Verifies Go module integrity and performs security scans with Trivy.
- Implements concurrency control to cancel redundant checks on rapid pushes, keeping the CI queue clean.

Additionally, this commit includes:
- A new "Test Build" status badge in `README.md` for immediate feedback on code health.
- A fix for the existing "Release" badge, pointing it to the correct `production_release.yml` workflow.
Co-authored-by: qodo-merge-pro[bot] <151058649+qodo-merge-pro[bot]@users.noreply.github.com>
Co-authored-by: qodo-merge-pro[bot] <151058649+qodo-merge-pro[bot]@users.noreply.github.com>
Co-authored-by: qodo-merge-pro[bot] <151058649+qodo-merge-pro[bot]@users.noreply.github.com>
Co-authored-by: qodo-merge-pro[bot] <151058649+qodo-merge-pro[bot]@users.noreply.github.com>
…rkflows-36ca

Refactor and unify testing workflows
Co-authored-by: qodo-merge-pro[bot] <151058649+qodo-merge-pro[bot]@users.noreply.github.com>
Co-authored-by: qodo-merge-pro[bot] <151058649+qodo-merge-pro[bot]@users.noreply.github.com>
Co-authored-by: qodo-merge-pro[bot] <151058649+qodo-merge-pro[bot]@users.noreply.github.com>
Co-authored-by: qodo-merge-pro[bot] <151058649+qodo-merge-pro[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
* fix: address security vulnerabilities and code quality issues

- Fix command injection vulnerability in test-release.sh by adding -- separator for exec
- Add input validation to prevent malicious arguments in test-release.sh
- Make Version variable immutable by using GetVersion() function
- Fix inconsistent quote style in index.mjs comments

* fix: CI workflow shell formatting and .gitignore cleanup

- Fix broken shell line continuation in test_release.yml go build commands
- Remove excessive whitespace in go build commands (lines 103-104, 158-159)
- Use proper line continuation syntax with backslashes
- Remove redundant test-build/* line from .gitignore

* Update test-release.sh

Co-authored-by: qodo-merge-pro[bot] <151058649+qodo-merge-pro[bot]@users.noreply.github.com>

* Update main.go

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* Update main.go

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* fix: update ldflags to use lowercase 'main.version' variable

Updated all build commands in GitHub workflows to reference main.version instead of main.Version to match the renamed variable in main.go

---------

Co-authored-by: Cursor Agent <[email protected]>
Co-authored-by: qodo-merge-pro[bot] <151058649+qodo-merge-pro[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: qodo-merge-pro[bot] <151058649+qodo-merge-pro[bot]@users.noreply.github.com>
…build commands

- Removed input validation for arguments to simplify the script.
- Updated shell detection logic to directly execute zsh or bash without argument escaping.
- Changed method of retrieving project version from package.json using grep instead of node command.
- Adjusted dependency check logic to ensure yarn.lock is only considered if package-lock.json is absent.
- Simplified build commands by removing unnecessary ldflags settings.

Deleted yarn.lock and GitHub workflows README.md as part of cleanup.
@zkbkb zkbkb marked this pull request as draft July 26, 2025 02:29
@qodo-code-review
Copy link
Copy Markdown

qodo-code-review bot commented Jul 26, 2025

PR Reviewer Guide 🔍

(Review updated until commit b5a00bc)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 No relevant tests
🔒 Security concerns

Command injection:
The test-release.sh script executes multiple shell commands with user-controlled input and environment variables without proper sanitization. Variables like GO_EXECUTABLE and platform strings are used directly in command execution, which could lead to command injection if manipulated.

⚡ Recommended focus areas for review

Shell Compatibility

Complex shell detection logic with exec calls and array usage may fail on different systems. The script assumes bash/zsh features but starts with basic sh, potentially causing compatibility issues.

if [ -z "${BASH_VERSION}" -a -z "${ZSH_VERSION}" ]; then
    # We're in a basic shell, try to find zsh or bash
    if command -v zsh >/dev/null 2>&1; then
        exec zsh "$0" "$@"
    elif command -v bash >/dev/null 2>&1; then
        exec bash "$0" "$@"
    else
        echo "Error: Neither zsh nor bash found. Please install one of them."
        exit 1
    fi
fi
Error Handling

Multiple command executions lack proper error handling and may fail silently. The script uses pipefail but doesn't consistently check command results before proceeding.

local version
if ! version=$(node -p "require('./package.json').version" 2>/dev/null); then
    echo -e "${RED}[✗] Node.js: Invalid package.json or version not found${NC}"
    VERIFY_NODE_STATUS="failed"
    return 1
fi

# Check lock file strategy
if [ -f "yarn.lock" ] && [ ! -f "package-lock.json" ]; then
    if command -v yarn >/dev/null 2>&1; then
        echo "[${GREEN}${NC}] Node.js: Using ${BOLD}yarn${NC} (yarn.lock detected, version: ${BOLD}$version${NC})"
    else
        echo "${YELLOW}[!] Node.js: yarn.lock detected but yarn not found, will use"
        echo "${YELLOW}    will use ${BOLD}npm install${NC} (version: ${BOLD}$version${NC})${NC}"
        has_warnings=true
    fi
elif [ -f "package-lock.json" ]; then
    echo "[${GREEN}${NC}] Node.js: Using ${BOLD}npm ci${NC} (package-lock.json detected, version: ${BOLD}$version${NC})"
else
    echo "${YELLOW}[!] Node.js: No lock file found, will use"
    echo "${YELLOW}    will use ${BOLD}npm install${NC} (version: ${BOLD}$version${NC})${NC}"
    has_warnings=true
fi

# Test dependency installation (dry run)
if [ -f "yarn.lock" ] && command -v yarn >/dev/null 2>&1; then
    if ! output=$(yarn install --dry-run 2>&1); then
        echo "${RED}[✗] Node.js: Dependency check failed (yarn dry-run)${NC}"
Version Injection

The version variable is mutable and the GetVersion function is unnecessary for a simple variable access. Consider making version immutable or removing the getter function.

// Version is set at build time via ldflags
var version = "dev"

// GetVersion returns the application version (immutable)
func GetVersion() string {
	return version
}

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Summary of Changes

Hello @zkbkb, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request is a foundational set of changes aimed at improving the project's development and release infrastructure. My focus was on standardizing code quality, streamlining dependency management, and significantly enhancing the automated build and release testing capabilities across multiple platforms. These changes will contribute to a more consistent developer experience and a more reliable delivery process.

Highlights

  • Development Environment Setup: I've added an ESLint configuration (.eslintrc.json) to standardize JavaScript/Node.js code style and enforce quality checks. This includes rules for unused variables, console logs, semicolons, and quote usage.
  • Dependency Management Transition: I've transitioned the Node.js dependency management from Yarn to npm by removing yarn.lock and updating package.json to include npm linting scripts. The package-lock.json file is now ignored, indicating a preference for npm install or npm ci without committing the lock file.
  • Robust Build and Release Process: A significant new test-release.sh script has been introduced. This script automates comprehensive environment checks, Go module consistency verification, native binary builds, and cross-compilation tests for various platforms (Linux, macOS, Windows), greatly enhancing our release pipeline's reliability. I've also updated main.go to support build-time versioning for the Go binary and cleaned up go.mod dependencies.
  • Project Visibility and Performance: The README.md has been updated with new badges linking to CI/CD workflows and release information, improving project visibility. Additionally, I've optimized the repository scanning process in index.mjs by adding more common build and vendor directories to the skip list, which should improve performance.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review bot commented Jul 26, 2025

PR Code Suggestions ✨

Latest suggestions up to b5a00bc
Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Replace deprecated shell test operator

The -a flag in the test condition is deprecated and not POSIX compliant. Use -z
with logical AND operator && instead for better portability across different
shell implementations.

test-release.sh [4-14]

-if [ -z "${BASH_VERSION}" -a -z "${ZSH_VERSION}" ]; then
+if [ -z "${BASH_VERSION}" ] && [ -z "${ZSH_VERSION}" ]; then
     # We're in a basic shell, try to find zsh or bash
     if command -v zsh >/dev/null 2>&1; then
         exec zsh "$0" "$@"
     elif command -v bash >/dev/null 2>&1; then
         exec bash "$0" "$@"
     else
         echo "Error: Neither zsh nor bash found. Please install one of them."
         exit 1
     fi
 fi
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies the use of a deprecated and non-POSIX compliant -a operator and proposes using the modern, more portable && operator, which improves code quality.

Low
Remove unreachable code after loop

The function has an unreachable echo statement after the infinite while true
loop. This dead code should be removed as it will never execute and may confuse
maintainers.

test-release.sh [454-467]

 prompt_user() {
     while true; do
         read -p " " -n 1 -r
         echo
         if [[ $REPLY =~ ^[Yy]$ ]]; then
             return 0
         elif [[ $REPLY =~ ^[Nn]$ ]]; then
             return 1
         else
             echo "${YELLOW}Please answer y or n${NC}"
         fi
     done
-    echo
 }
  • Apply / Chat
Suggestion importance[1-10]: 4

__

Why: The suggestion correctly identifies and removes an unreachable echo statement, which is a good practice for code cleanup and maintainability.

Low
  • More

Previous suggestions

Suggestions up to commit 2687307
CategorySuggestion                                                                                                                                    Impact
General
Replace deprecated test operator

The -a operator in the test condition is deprecated and not POSIX compliant. Use
&& instead for better portability across different shell implementations.

test-release.sh [3-14]

 # Smart shell detection: try zsh first, then bash
-if [ -z "${BASH_VERSION}" -a -z "${ZSH_VERSION}" ]; then
+if [ -z "${BASH_VERSION}" ] && [ -z "${ZSH_VERSION}" ]; then
     # We're in a basic shell, try to find zsh or bash
     if command -v zsh >/dev/null 2>&1; then
         exec zsh "$0" "$@"
     elif command -v bash >/dev/null 2>&1; then
         exec bash "$0" "$@"
     else
         echo "Error: Neither zsh nor bash found. Please install one of them."
         exit 1
     fi
 fi
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies that -a is a deprecated and non-POSIX compliant operator and proposes using the modern && syntax, which improves script portability and adheres to best practices.

Low

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces significant enhancements to the project's development and deployment workflow, including CI/CD integration, versioning for Go binaries, a local release testing script, and ESLint for code quality. The changes are generally well-implemented.

My review focuses on a few key areas to improve robustness and maintainability:

  • Ensuring reproducible builds by correctly managing package-lock.json.
  • Correcting the release test script to properly inject the version number during builds.
  • Addressing minor issues in configuration files and scripts for consistency and correctness.

Overall, this is a great step forward for the project's maturity.

}

# Build phase
run_build_phase() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The run_build_phase function builds the Go binaries but doesn't inject the version number using -ldflags. This is a key part of a release process, and the --version flag test will not be representative of a real release build as it will always report "dev".

You should extract the version from package.json into a variable at the start of the function, construct the ldflags string, and then use this variable in both the native and cross-compilation go build commands.

"*.go",
"git-status-dash-*"
]
} No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

It's a common convention to end files with a newline character. Some tools and editors might have issues with files that don't have a trailing newline.

}

"lint": "eslint --ext .js,.mjs .",
"lint:ci": "eslint --ext .js,.mjs . --quiet",
"lint:fix": "eslint --ext .js,.mjs . --fix",
"test": "echo \"No tests specified\" && exit 0"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The test script currently does nothing and exits with a success code. This can be misleading in CI/CD pipelines, as it will always report success for the test stage. It would be better to either add some basic tests or have the script exit with an error to indicate that no tests are implemented.

    "test": "echo \"Error: no test specified\" && exit 1"

echo "${YELLOW}Please answer y or n${NC}"
fi
done
echo
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

This echo command is unreachable because the while true loop above it can only be exited via a return statement.

@zkbkb
Copy link
Copy Markdown
Owner Author

zkbkb commented Jul 26, 2025

/describe

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review bot commented Jul 26, 2025

PR Description updated to latest commit (b5a00bc)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

File Walkthrough

@zkbkb zkbkb marked this pull request as ready for review July 26, 2025 02:38
@zkbkb zkbkb merged commit 344e503 into main Jul 26, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants