Skip to content

Fix: issue #4 and #5#6

Merged
versenilvis merged 10 commits into
mainfrom
fix/issue-4-5
May 23, 2026
Merged

Fix: issue #4 and #5#6
versenilvis merged 10 commits into
mainfrom
fix/issue-4-5

Conversation

@versenilvis

Copy link
Copy Markdown
Owner
  • Issue [BUG] Install #4: currently there is no official release yet so we cant download from installer

  • I added an installer debugger here to test (bb9a750)

  • Issue [BUG] Crash #5: my another library fuzzy has a logic bug so it causes crash, fix 8ad579c

  • Other:

    • remove unneeded comments (3ba87b2)
    • Add some tests
    • Fix reload crash (d801307)
    • New optimized build command (7b63014)

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

Copy link
Copy Markdown

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 implements thread-safety for shell aliases using a sync.RWMutex and ensures system command scanning is idempotent via sync.Once. It also improves the install.sh script's error handling and adds a local testing target with a mock server to the justfile. Review feedback highlights a non-POSIX head flag in the installer that breaks on macOS, recommends reverting the GOAMD64 target to v3 for broader hardware compatibility, and notes that the wget path in the installer bypasses the new error handling logic.

Comment thread scripts/install.sh Outdated
"https://api.github.com/repos/${REPO}/releases/latest")
"${IRIS_API_URL}/repos/${REPO}/releases/latest")
http_code=$(echo "${http_response}" | tail -1)
releases=$(echo "${http_response}" | head -n -1)

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 -n -1 flag for head is a GNU extension and is not POSIX-compliant. This will cause the installer to fail on systems using BSD head (such as macOS) with an illegal line count -- -1 error. To ensure portability across different Unix-like systems, use sed '$d' to remove the last line of the output.

Suggested change
releases=$(echo "${http_response}" | head -n -1)
releases=$(echo "${http_response}" | sed '$d')

Comment thread justfile
[group('build')]
optimized-build:
@GOAMD64=v3 go build -pgo=auto -ldflags="-s -w" -trimpath -o iris main.go
@GOAMD64=v4 go build -ldflags="-s -w" -trimpath -o iris main.go

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

Targeting GOAMD64=v4 for the 'optimized' build significantly limits hardware compatibility, as it requires AVX-512 support which is absent in many modern consumer CPUs. This may lead to SIGILL (Illegal Instruction) crashes for users running the binary on older or mid-range hardware. v3 (AVX2) is generally considered the standard target for optimized x86_64 builds as it covers most CPUs from the last decade.

    @GOAMD64=v3 go build -ldflags="-s -w" -trimpath -o iris main.go

Comment thread scripts/install.sh Outdated
${GITHUB_TOKEN:+--header "Authorization: Bearer ${GITHUB_TOKEN}"} \
"https://api.github.com/repos/${REPO}/releases/latest")
"${IRIS_API_URL}/repos/${REPO}/releases/latest")
http_code="200"

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

In the wget code path, the http_code is hardcoded to 200. This bypasses the specific error handling logic implemented for curl (lines 108-114), which provides helpful messages for 404 (no releases) or 403 (rate limiting) errors. While set -e will stop the script on a wget failure, the user will miss the context-specific error messages.

@versenilvis versenilvis merged commit 2d89460 into main May 23, 2026
2 checks passed
@versenilvis versenilvis mentioned this pull request May 23, 2026
versenilvis added a commit that referenced this pull request May 23, 2026
Merged #6
- crash log (fd4e97f)
- perf(watchdog): limit buffer size and optimize crash substring search (e53b3b8)
- refactor(crash): use dynamic buffer for stack trace capture (b564959)
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.

1 participant