Skip to content

fix: escape file path segments in GetFileContent for special characters#481

Merged
agnivo988 merged 3 commits into
agnivo988:mainfrom
saurabhhhcodes:fix/unescaped-file-paths-432
Jun 19, 2026
Merged

fix: escape file path segments in GetFileContent for special characters#481
agnivo988 merged 3 commits into
agnivo988:mainfrom
saurabhhhcodes:fix/unescaped-file-paths-432

Conversation

@saurabhhhcodes

@saurabhhhcodes saurabhhhcodes commented Jun 13, 2026

Copy link
Copy Markdown

GetFileContent constructed a raw URL from the file path, which caused 404 errors when paths contained spaces, hashes, or non-ASCII characters.

Added url.PathEscape to each path segment before URL construction.

Closes #432

Summary by CodeRabbit

  • Documentation

    • Updated README with Table of Contents, prerequisites, and clearer installation and usage instructions
    • Added contribution workflow and development setup guidance
  • Features

    • Enhanced repository maturity assessment to account for GitHub releases
    • Improved accuracy of repository analysis and comparison
  • Bug Fixes

    • Fixed path URL escaping in GitHub file content retrieval

@coderabbitai

coderabbitai Bot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds GitHub releases detection to maturity scoring across all repository analysis entry points, fixes URL path escaping for file lookups with special characters, and refreshes documentation with contributing guidelines and contributor credits.

Changes

Releases detection and maturity scoring integration

Layer / File(s) Summary
GitHub client releases API and path escaping
internal/github/releases.go, internal/github/client.go
New HasReleases(owner, repo) method queries the Releases API, caches results with singleflight deduplication. GetFileContent now URL-escapes path segments to handle special characters in file names.
Comparison infrastructure: CompareInput schema and data flow
internal/output/compare.go, cmd/compare.go
CompareInput struct gains HasReleases field. fetchCompareInput queries HasReleases for each repository and populates the field in returned comparison data. buildCompareRepository passes HasReleases to maturity scoring.
Maturity scoring with releases detection across entry points
cmd/analyze.go, internal/analyzer/certificate.go, internal/scheduler/scheduler.go, internal/ui/app.go
Replace hard-coded false releases parameter in RepoMaturityScore calls with actual HasReleases lookups. Integrated into analyze command, certificate generation, scheduler jobs, and UI flows (single-repo analysis and comparison).

Documentation refresh and contribution guidelines

Layer / File(s) Summary
README badges, quick-start, contributing guide, and contributors
README.md
Expand header with status badges and table of contents; add Go 1.21+ and GitHub token prerequisites; revise installation and verification steps. Add detailed fork/branch/test/commit workflow with development setup and code style guidance. Update contributor avatars/links and retain MIT license.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • agnivo988/Repo-lyzer#155: Both PRs update the "Maintainers & Contributors" section of README.md with different contributor listings and community guidance.
  • agnivo988/Repo-lyzer#100: Retrieved PR introduces maturity scoring with hard-coded false for releases; this PR replaces that placeholder with actual HasReleases detection.
  • agnivo988/Repo-lyzer#52: New HasReleases implementation depends on the Client.get() helper method introduced in the retrieved PR.

Suggested labels

type:bug, quality:clean

Poem

🐰 A rabbit hops through releases with glee,
Escaping paths where special chars roam free,
Maturity now knows when repos have tags,
No more hard-coded false in the bags!
With docs so clear, the burrow's complete. 🌟

🚥 Pre-merge checks | ✅ 1 | ❌ 4

❌ Failed checks (3 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR addresses issue #432 by implementing the proposed fix to escape file path segments in GetFileContent using net/url.PathEscape. However, it also includes substantial unrelated changes for HasReleases functionality across 7 additional files. The PR includes scope creep beyond the linked issue. Consider splitting HasReleases additions into a separate PR to keep the fix for issue #432 focused and reviewable.
Out of Scope Changes check ⚠️ Warning The PR includes extensive HasReleases functionality additions (client.go HasReleases method, releases.go, compare.go, certificate.go, scheduler.go, app.go, and README.md updates) that are not mentioned in or required by issue #432. Remove HasReleases-related changes from this PR or clarify their connection to issue #432. These changes appear to be a separate feature and should be submitted as a distinct pull request.
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The PR title clearly describes the main change: escaping file path segments in GetFileContent to handle special characters. However, the actual PR includes significantly broader changes across multiple files beyond just the file path escaping fix. Verify whether the title should reflect only the primary GetFileContent fix or be updated to indicate the broader HasReleases feature additions across the codebase.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cmd/compare.go`:
- Around line 160-163: The HasReleases error handling should be lenient like
other entry points: if client.HasReleases(owner, repo) returns an error, set
hasReleases = false, log a warning with the error (don’t return), and continue;
update the block around the HasReleases call (the hasReleases variable and its
error check) to swallow the error, emit a warning using the existing logger (or
fmt.Printf if none), and default hasReleases to false so the compare command
behavior aligns with analyze/certificate/scheduler/UI.

In `@README.md`:
- Around line 42-44: Update the README.md entry that currently reads "A [GitHub
Personal Access Token](https://github.com/settings/tokens) (required for API
calls)" and make it accurate by changing it to "A [GitHub Personal Access
Token](https://github.com/settings/tokens) (optional; recommended to avoid
GitHub API rate limits)" so the prerequisite wording aligns with
docs/CONTRIBUTING.md and reflects that the token is recommended but not strictly
required.
- Around line 52-54: The README currently shows "repo-lyzer version" but Cobra's
Version field (set in cmd/root.go) only provides the automatic --version flag
and there is no registered version subcommand via rootCmd.AddCommand(...), so
update README.md to use "repo-lyzer --version" or implement a real subcommand:
add a new version command (e.g., versionCmd) that prints the version and
register it with rootCmd.AddCommand(versionCmd) (or alternatively adjust
cmd/root.go to expose a subcommand that delegates to the existing Version
behavior).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6eaa1ae1-0800-4f1c-8f8e-b0ccda74fd6a

📥 Commits

Reviewing files that changed from the base of the PR and between 6d6fa89 and 76c907b.

📒 Files selected for processing (9)
  • README.md
  • cmd/analyze.go
  • cmd/compare.go
  • internal/analyzer/certificate.go
  • internal/github/client.go
  • internal/github/releases.go
  • internal/output/compare.go
  • internal/scheduler/scheduler.go
  • internal/ui/app.go

Comment thread cmd/compare.go
Comment on lines +160 to +163
hasReleases, err := client.HasReleases(owner, repo)
if err != nil {
return output.CompareInput{}, fmt.Errorf("error checking releases for %s/%s: %w", owner, repo, err)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Consider aligning error handling with other entry points.

The compare command fails the entire comparison if HasReleases returns an error, while other entry points (analyze, certificate, scheduler, UI) silently ignore the error and default to hasReleases = false. This creates inconsistent behavior across the application.

Consider either:

  1. Making all entry points lenient (ignore the error, log a warning, default to false)
  2. Making all entry points strict (fail on error)

Lenient handling is likely more appropriate since releases detection is supplementary to core metrics and shouldn't block the primary analysis flow.

🔧 Proposed fix to align with lenient pattern
-	hasReleases, err := client.HasReleases(owner, repo)
-	if err != nil {
-		return output.CompareInput{}, fmt.Errorf("error checking releases for %s/%s: %w", owner, repo, err)
-	}
+	hasReleases, _ := client.HasReleases(owner, repo)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
hasReleases, err := client.HasReleases(owner, repo)
if err != nil {
return output.CompareInput{}, fmt.Errorf("error checking releases for %s/%s: %w", owner, repo, err)
}
hasReleases, _ := client.HasReleases(owner, repo)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/compare.go` around lines 160 - 163, The HasReleases error handling should
be lenient like other entry points: if client.HasReleases(owner, repo) returns
an error, set hasReleases = false, log a warning with the error (don’t return),
and continue; update the block around the HasReleases call (the hasReleases
variable and its error check) to swallow the error, emit a warning using the
existing logger (or fmt.Printf if none), and default hasReleases to false so the
compare command behavior aligns with analyze/certificate/scheduler/UI.

Comment thread README.md
Comment on lines +42 to +44
### Prerequisites
- **Go 1.21+** (for `go install`) or **Docker** (for containerized usage)
- A [GitHub Personal Access Token](https://github.com/settings/tokens) (required for API calls)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

GitHub token prerequisite marked "required" but documentation says "optional".

Line 44 states the GitHub Personal Access Token is "required for API calls", but docs/CONTRIBUTING.md explicitly marks it as "Optional" for avoiding rate limits. The GitHub API has a rate limit for unauthenticated requests (~60 req/hr), so the token is recommended but not strictly required. Clarify or align the language to avoid misleading new contributors.

🔧 Suggested clarification

Change line 44 from:

- A [GitHub Personal Access Token](https://github.com/settings/tokens) (required for API calls)

To:

- A [GitHub Personal Access Token](https://github.com/settings/tokens) (optional; recommended to avoid GitHub API rate limits)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@README.md` around lines 42 - 44, Update the README.md entry that currently
reads "A [GitHub Personal Access Token](https://github.com/settings/tokens)
(required for API calls)" and make it accurate by changing it to "A [GitHub
Personal Access Token](https://github.com/settings/tokens) (optional;
recommended to avoid GitHub API rate limits)" so the prerequisite wording aligns
with docs/CONTRIBUTING.md and reflects that the token is recommended but not
strictly required.

Comment thread README.md
Comment on lines +52 to 54
```bash
repo-lyzer version
```

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify the version command is defined in the CLI

# Search for version command definition in cmd/ or main.go
rg -n --type=go -A5 -B1 'version.*command|cmd.*version' --glob '!vendor' || echo "No version command found"

# Alternatively, check for a version flag or subcommand
ast-grep --pattern 'flag.String|func.*version|const.*Version'

Repository: agnivo988/Repo-lyzer

Length of output: 379


🏁 Script executed:

#!/bin/bash
# Search for an actual "version" subcommand definition and wiring in Cobra
rg -n --glob 'cmd/**/*.go' --glob '!vendor/**' \
  'Use:\s*["'\'']version["'\'']|versionCmd|NewVersion|AddCommand\(.+version|AddCommand\(\s*version' || true

# Look for how rootCmd wires subcommands
rg -n --glob 'cmd/root.go' 'AddCommand\(' || true

# Show root.go area where subcommands are added (small, targeted)
sed -n '1,220p' cmd/root.go | nl -ba | sed -n '1,220p'

Repository: agnivo988/Repo-lyzer

Length of output: 106


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Search for cobra 'version' subcommand wiring =="
rg -n --glob 'cmd/**/*.go' --glob '!vendor/**' \
  'Use:\s*["'\'']version["'\'']|versionCmd|NewVersion|AddCommand\(.+version|AddCommand\(\s*version' || true

echo "== Search for any file containing 'version' subcommand handler patterns =="
rg -n --glob 'cmd/**/*.go' --glob '!vendor/**' \
  '\bversion\b.*(Command|cmd|cobra|Use:|Args|RunE|Run)' || true

echo "== Show cmd/root.go around subcommand additions =="
# Print whole file if small, else just first ~250 lines
if [ "$(wc -l < cmd/root.go)" -le 250 ]; then
  cat -n cmd/root.go
else
  sed -n '1,250p' cmd/root.go | cat -n
fi

echo "== List cmd directory to locate likely subcommand files =="
ls -la cmd || true

Repository: agnivo988/Repo-lyzer

Length of output: 3837


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== README snippet around 'repo-lyzer version' =="
sed -n '40,80p' README.md | cat -n

echo "== Search for cobra subcommand wiring (AddCommand) =="
rg -n --glob 'cmd/**/*.go' --glob '!vendor/**' 'AddCommand\(' cmd || true

echo "== Search for an explicit 'version' subcommand (Use: \"version\") =="
rg -n --glob 'cmd/**/*.go' --glob '!vendor/**' 'Use:\s*["'\'']version["'\'']' cmd || true

echo "== Search for any cobra command named version (func/var/file patterns) =="
rg -n --glob 'cmd/**/*.go' --glob '!vendor/**' '\bversion\b' cmd || true

echo "== Cobra version in go.mod =="
cat go.mod | rg -n 'github\.com/spf13/cobra' || true

Repository: agnivo988/Repo-lyzer

Length of output: 3361


🌐 Web query:

Cobra v1.10.2 Command.Version default "version" subcommand or --version flag

💡 Result:

In Cobra v1.10.2, there is no default "version" subcommand provided by the library [1]. The library does, however, provide an automatic "--version" flag [1]. 1. --version Flag Cobra automatically adds a top-level "--version" flag to your application if you set the Version field on the root command [1]. When a user executes the application with this flag, Cobra prints the version string to standard output using a default template [1]. You can customize this template by using the cmd.SetVersionTemplate(s string) function [1]. This flag functionality is explicitly implemented in the Cobra command structure, as seen in the source code [2][3]. 2. "version" Subcommand Cobra does not provide a default "version" subcommand [1]. If you require a "version" command that behaves like a subcommand (e.g., app version), you must implement it manually by creating a new cobra.Command and adding it to your root command using rootCmd.AddCommand(versionCmd) [1][4]. Summary of differences: - --version Flag: Automatically added if the Version field is set [1]. It is handled by the library internally via InitDefaultVersionFlag [5][3]. - version Subcommand: Not provided by default; requires manual implementation [1][4].

Citations:


Update README: use repo-lyzer --version (or implement a version subcommand).

cmd/root.go sets Cobra’s Version field, which provides an automatic --version flag—not a version subcommand; there’s no registered version command found under rootCmd.AddCommand(...), so repo-lyzer version will likely fail with “unknown command”. Update README.md (lines 52-54) accordingly or add a real version subcommand.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@README.md` around lines 52 - 54, The README currently shows "repo-lyzer
version" but Cobra's Version field (set in cmd/root.go) only provides the
automatic --version flag and there is no registered version subcommand via
rootCmd.AddCommand(...), so update README.md to use "repo-lyzer --version" or
implement a real subcommand: add a new version command (e.g., versionCmd) that
prints the version and register it with rootCmd.AddCommand(versionCmd) (or
alternatively adjust cmd/root.go to expose a subcommand that delegates to the
existing Version behavior).

@agnivo988
agnivo988 merged commit a9c3a6a into agnivo988:main Jun 19, 2026
5 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.

[BUG]: Unescaped file paths in GetFileContent cause 404 errors on files with special characters

2 participants