fix(gitea): normalize "." path to empty string in ListContents #72

Merged
aweiker merged 2 commits from issue-70 into main 2026-05-11 14:16:22 +00:00
Owner

Summary

Fixes #70patterns-files="." causes 500 error from Gitea API.

Problem

When patterns-files is set to ., the client passes it directly to the Gitea contents API:

GET /api/v1/repos/owner/repo/contents/.

Gitea rejects this with HTTP 500 because . is considered a malformed path component.

Solution

Normalize . to empty string at the start of ListContents(). This makes . behave identically to "", which correctly lists the repository root.

Testing

  • Added TestListContents_DotPath that verifies . is normalized to the root contents endpoint
  • All existing tests pass
## Summary Fixes #70 — `patterns-files="."` causes 500 error from Gitea API. ## Problem When `patterns-files` is set to `.`, the client passes it directly to the Gitea contents API: ``` GET /api/v1/repos/owner/repo/contents/. ``` Gitea rejects this with HTTP 500 because `.` is considered a malformed path component. ## Solution Normalize `.` to empty string at the start of `ListContents()`. This makes `.` behave identically to `""`, which correctly lists the repository root. ## Testing - Added `TestListContents_DotPath` that verifies `.` is normalized to the root contents endpoint - All existing tests pass
rodin added 1 commit 2026-05-11 13:24:19 +00:00
fix(gitea): normalize "." path to empty string in ListContents
CI / test (pull_request) Successful in 18s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 26s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 34s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 43s
e6b1840ffc
Gitea API rejects "." with HTTP 500 (malformed path component).
When patterns-files is set to ".", normalize it to empty string
before making the API call.

Fixes #70
sonnet-review-bot approved these changes 2026-05-11 13:24:53 +00:00
Dismissed
sonnet-review-bot left a comment
First-time contributor

Original review

Supersededsee current review for up-to-date findings.

Previous findings (commit e6b1840f)

Sonnet Review

Summary

Minimal, correct fix for a well-defined bug. The normalization is placed at the right boundary (entry point of ListContents), the comment explains the why, and the test directly verifies the URL path correction.

Recommendation

APPROVE — The change is correct and idiomatic. Normalizing '.' to '' at the start of ListContents is the right place — it's a single guard clause that handles a known-bad input before it reaches URL construction. CI passes. Approve as-is.


Review by sonnet


Evaluated against e6b1840f

~~Original review~~ **Superseded** — [see current review](https://gitea.weiker.me/rodin/review-bot/pulls/72#pullrequestreview-2522) for up-to-date findings. <details><summary>Previous findings (commit e6b1840f)</summary> # Sonnet Review ## Summary Minimal, correct fix for a well-defined bug. The normalization is placed at the right boundary (entry point of ListContents), the comment explains the why, and the test directly verifies the URL path correction. ## Recommendation **APPROVE** — The change is correct and idiomatic. Normalizing '.' to '' at the start of ListContents is the right place — it's a single guard clause that handles a known-bad input before it reaches URL construction. CI passes. Approve as-is. --- *Review by sonnet* <!-- review-bot:sonnet --> --- *Evaluated against e6b1840f* </details> <!-- review-bot:sonnet -->
Author
Owner

Self-review: PASS

Coherence: Single-purpose fix — normalizes . to empty string. No scope creep.

Fit: Matches existing pattern (empty string handling already exists at line 443). Comment explains the why.

Solution: Simplest fix per issue #70. . should behave identically to "" (both mean repo root).

Completeness: Test added (TestListContents_DotPath) that verifies the normalization and fails without the fix.

No issues found — ready for human review.

## Self-review: PASS **Coherence**: Single-purpose fix — normalizes `.` to empty string. No scope creep. **Fit**: Matches existing pattern (empty string handling already exists at line 443). Comment explains the why. **Solution**: Simplest fix per issue #70. `.` should behave identically to `""` (both mean repo root). **Completeness**: Test added (`TestListContents_DotPath`) that verifies the normalization and fails without the fix. No issues found — ready for human review.
security-review-bot requested review from security-review-bot 2026-05-11 13:25:12 +00:00
security-review-bot approved these changes 2026-05-11 13:25:12 +00:00
Dismissed
security-review-bot left a comment
Collaborator

Original review

Supersededsee current review for up-to-date findings.

Previous findings (commit e6b1840f)

Security Review

Summary

The change safely normalizes a '.' path to an empty string in ListContents and adds a corresponding test. There are no security implications introduced by this update.

Recommendation

APPROVE — CI passed and the modification is minimal and benign from a security perspective. The path normalization does not weaken input validation or escaping, and all URL components remain properly escaped. Approve the PR.


Review by security


Evaluated against e6b1840f

~~Original review~~ **Superseded** — [see current review](https://gitea.weiker.me/rodin/review-bot/pulls/72#pullrequestreview-2526) for up-to-date findings. <details><summary>Previous findings (commit e6b1840f)</summary> # Security Review ## Summary The change safely normalizes a '.' path to an empty string in ListContents and adds a corresponding test. There are no security implications introduced by this update. ## Recommendation **APPROVE** — CI passed and the modification is minimal and benign from a security perspective. The path normalization does not weaken input validation or escaping, and all URL components remain properly escaped. Approve the PR. --- *Review by security* <!-- review-bot:security --> --- *Evaluated against e6b1840f* </details> <!-- review-bot:security -->
rodin added the self-reviewed label 2026-05-11 13:25:18 +00:00
gpt-review-bot approved these changes 2026-05-11 13:25:22 +00:00
gpt-review-bot left a comment
First-time contributor

Original review

Supersededsee current review for up-to-date findings.

Previous findings (commit e6b1840f)

Gpt Review

Summary

The change correctly normalizes a dot path to the repository root for Gitea contents API calls and adds a focused test verifying the behavior. The implementation is minimal, idiomatic, and aligns with the stated problem; all tests pass.

Recommendation

APPROVE — This is a well-scoped fix for the reported issue: normalizing '.' to an empty path avoids Gitea's 500 error and preserves expected semantics. The accompanying test effectively guards against regressions. As a non-blocking enhancement, consider normalizing additional dot-segment variants (e.g., './', 'path/.') if they can surface from user inputs, potentially by applying a conservative clean for the root-only case, but this can be addressed separately. Approving as is.


Review by gpt


Evaluated against e6b1840f

~~Original review~~ **Superseded** — [see current review](https://gitea.weiker.me/rodin/review-bot/pulls/72#pullrequestreview-2523) for up-to-date findings. <details><summary>Previous findings (commit e6b1840f)</summary> # Gpt Review ## Summary The change correctly normalizes a dot path to the repository root for Gitea contents API calls and adds a focused test verifying the behavior. The implementation is minimal, idiomatic, and aligns with the stated problem; all tests pass. ## Recommendation **APPROVE** — This is a well-scoped fix for the reported issue: normalizing '.' to an empty path avoids Gitea's 500 error and preserves expected semantics. The accompanying test effectively guards against regressions. As a non-blocking enhancement, consider normalizing additional dot-segment variants (e.g., './', 'path/.') if they can surface from user inputs, potentially by applying a conservative clean for the root-only case, but this can be addressed separately. Approving as is. --- *Review by gpt* <!-- review-bot:gpt --> --- *Evaluated against e6b1840f* </details> <!-- review-bot:gpt -->
rodin added the ready label 2026-05-11 13:25:23 +00:00
aweiker was assigned by rodin 2026-05-11 13:25:24 +00:00
aweiker requested changes 2026-05-11 14:03:01 +00:00
Dismissed
@@ -437,1 +437,4 @@
func (c *Client) ListContents(ctx context.Context, owner, repo, path string) ([]ContentEntry, error) {
// Normalize "." to empty string — Gitea API rejects "." with 500
if path == "." {
path = ""
Contributor

Should this be '""' or '/'? In Rodin we needed to use a '/' as a '""' caused it to default to README.md

Should this be '""' or '/'? In Rodin we needed to use a '/' as a '""' caused it to default to README.md
rodin was assigned by aweiker 2026-05-11 14:03:13 +00:00
Author
Owner

Re: Should this be "" or /?

Empty string is correct here. In ListContents, empty string produces:

/api/v1/repos/{owner}/{repo}/contents

(no path suffix), which Gitea handles correctly and returns an array.

The reason gargoyle needed / was different: the action.yml has default: 'README.md' for patterns-files. So when the workflow passes patterns-files: '', the action treats it as unset and substitutes the default. That's issue #71 (bad default).

This fix (normalizing ."") handles the case where someone explicitly passes . meaning "repo root". Both . and "" should produce the same API call:

  • ""/contents
  • . → (after this fix) /contents
  • //contents/ (also works, but trailing slash is unnecessary)
Re: Should this be `""` or `/`? Empty string is correct here. In `ListContents`, empty string produces: ``` /api/v1/repos/{owner}/{repo}/contents ``` (no path suffix), which Gitea handles correctly and returns an array. The reason gargoyle needed `/` was different: the **action.yml** has `default: 'README.md'` for `patterns-files`. So when the workflow passes `patterns-files: ''`, the action treats it as unset and substitutes the default. That's issue #71 (bad default). This fix (normalizing `.` → `""`) handles the case where someone explicitly passes `.` meaning "repo root". Both `.` and `""` should produce the same API call: - `""` → `/contents` ✓ - `.` → (after this fix) `/contents` ✓ - `/` → `/contents/` (also works, but trailing slash is unnecessary)
sonnet-review-bot approved these changes 2026-05-11 14:11:42 +00:00
Dismissed
sonnet-review-bot left a comment
First-time contributor

Original review

Supersededsee current review for up-to-date findings.

Previous findings (commit b0349a22)

Sonnet Review

Summary

Clean, minimal fix that normalizes '.' to empty string in ListContents before URL construction. The implementation is correct, well-placed, and accompanied by a focused unit test that verifies the exact behavior. The CI workflow addition provides an end-to-end smoke test for the fix.

Findings

# Severity File Line Finding
1 [NIT] .gitea/workflows/ci.yml 70 The test-dot-path CI job runs the full review-bot against a real patterns repo (rodin/security-patterns) on every PR. This is an integration test that consumes LLM API credits and external dependencies to verify what is already covered by the unit test TestListContents_DotPath. Once the fix is merged, this job has no ongoing value and becomes permanent noise. Consider removing it after the fix lands, or making it a one-off manual trigger rather than a permanent PR check.

Recommendation

APPROVE — Approve. The code change is correct and minimal — a 3-line guard at the top of ListContents with a clear comment, exactly where the normalization should live. The unit test is well-structured and directly verifies the URL path that gets hit. The only concern is the new CI job: it's a reasonable way to verify the fix end-to-end for this PR, but it should be cleaned up after merge since the unit test provides sufficient ongoing coverage. This is a NIT-level process concern, not a blocking issue.


Review by sonnet


Evaluated against b0349a22

~~Original review~~ **Superseded** — [see current review](https://gitea.weiker.me/rodin/review-bot/pulls/72#pullrequestreview-2527) for up-to-date findings. <details><summary>Previous findings (commit b0349a22)</summary> # Sonnet Review ## Summary Clean, minimal fix that normalizes '.' to empty string in ListContents before URL construction. The implementation is correct, well-placed, and accompanied by a focused unit test that verifies the exact behavior. The CI workflow addition provides an end-to-end smoke test for the fix. ## Findings | # | Severity | File | Line | Finding | |---|----------|------|------|--------| | 1 | [NIT] | `.gitea/workflows/ci.yml` | 70 | The `test-dot-path` CI job runs the full review-bot against a real patterns repo (rodin/security-patterns) on every PR. This is an integration test that consumes LLM API credits and external dependencies to verify what is already covered by the unit test `TestListContents_DotPath`. Once the fix is merged, this job has no ongoing value and becomes permanent noise. Consider removing it after the fix lands, or making it a one-off manual trigger rather than a permanent PR check. | ## Recommendation **APPROVE** — Approve. The code change is correct and minimal — a 3-line guard at the top of `ListContents` with a clear comment, exactly where the normalization should live. The unit test is well-structured and directly verifies the URL path that gets hit. The only concern is the new CI job: it's a reasonable way to verify the fix end-to-end for this PR, but it should be cleaned up after merge since the unit test provides sufficient ongoing coverage. This is a NIT-level process concern, not a blocking issue. --- *Review by sonnet* <!-- review-bot:sonnet --> --- *Evaluated against b0349a22* </details> <!-- review-bot:sonnet -->
@@ -67,1 +67,4 @@
run: ./review-bot
# Test dot path normalization with security-patterns repo
test-dot-path:
First-time contributor

[NIT] The test-dot-path CI job runs the full review-bot against a real patterns repo (rodin/security-patterns) on every PR. This is an integration test that consumes LLM API credits and external dependencies to verify what is already covered by the unit test TestListContents_DotPath. Once the fix is merged, this job has no ongoing value and becomes permanent noise. Consider removing it after the fix lands, or making it a one-off manual trigger rather than a permanent PR check.

**[NIT]** The `test-dot-path` CI job runs the full review-bot against a real patterns repo (rodin/security-patterns) on every PR. This is an integration test that consumes LLM API credits and external dependencies to verify what is already covered by the unit test `TestListContents_DotPath`. Once the fix is merged, this job has no ongoing value and becomes permanent noise. Consider removing it after the fix lands, or making it a one-off manual trigger rather than a permanent PR check.
gpt-review-bot approved these changes 2026-05-11 14:12:12 +00:00
gpt-review-bot left a comment
First-time contributor

Gpt Review

Summary

Change correctly normalizes '.' to the repository root in ListContents and adds a focused unit test. CI passed and the update aligns with error-handling and testing conventions.

Findings

# Severity File Line Finding
1 [MINOR] gitea/client.go 438 Consider normalizing other dot-equivalent root paths such as "./" (and possibly variants like ".//") to the empty string as well, to be robust against alternate inputs. A small follow-up (e.g., trimming a single leading "./") and an extra test would harden this further.

Recommendation

APPROVE — The fix is concise, correct, and well-tested. Normalizing '.' eliminates the Gitea 500 issue and the new unit test validates behavior. CI passed. As a minor enhancement, consider also handling './' to cover more user inputs that should map to the repo root, and add a corresponding test. Otherwise, this is good to merge.


Review by gpt


Evaluated against b0349a22

# Gpt Review ## Summary Change correctly normalizes '.' to the repository root in ListContents and adds a focused unit test. CI passed and the update aligns with error-handling and testing conventions. ## Findings | # | Severity | File | Line | Finding | |---|----------|------|------|--------| | 1 | [MINOR] | `gitea/client.go` | 438 | Consider normalizing other dot-equivalent root paths such as "./" (and possibly variants like ".//") to the empty string as well, to be robust against alternate inputs. A small follow-up (e.g., trimming a single leading "./") and an extra test would harden this further. | ## Recommendation **APPROVE** — The fix is concise, correct, and well-tested. Normalizing '.' eliminates the Gitea 500 issue and the new unit test validates behavior. CI passed. As a minor enhancement, consider also handling './' to cover more user inputs that should map to the repo root, and add a corresponding test. Otherwise, this is good to merge. --- *Review by gpt* <!-- review-bot:gpt --> --- *Evaluated against b0349a22*
@@ -435,6 +435,10 @@ type ContentEntry struct {
// ListContents lists files and directories at a given path in a repo.
// Pass an empty path to list the repository root.
func (c *Client) ListContents(ctx context.Context, owner, repo, path string) ([]ContentEntry, error) {
// Normalize "." to empty string — Gitea API rejects "." with 500
First-time contributor

[MINOR] Consider normalizing other dot-equivalent root paths such as "./" (and possibly variants like ".//") to the empty string as well, to be robust against alternate inputs. A small follow-up (e.g., trimming a single leading "./") and an extra test would harden this further.

**[MINOR]** Consider normalizing other dot-equivalent root paths such as "./" (and possibly variants like ".//") to the empty string as well, to be robust against alternate inputs. A small follow-up (e.g., trimming a single leading "./") and an extra test would harden this further.
gpt-review-bot approved these changes 2026-05-11 14:12:14 +00:00
gpt-review-bot left a comment
First-time contributor

Dot-path-test Review

Summary

Change cleanly normalizes a dot path to the repository root in ListContents and adds a focused unit test. CI passed and the implementation is small, safe, and consistent with existing patterns.

Findings

# Severity File Line Finding
1 [MINOR] gitea/client.go 435 Consider also normalizing equivalent root path variants such as "./" or "/" (if ever passed) to "" for robustness. Currently only exact "." is handled.
2 [NIT] .gitea/workflows/ci.yml 69 The new test-dot-path job introduces an external integration check that depends on secrets and a live repo. Since a unit test already covers the behavior, consider whether this job is necessary to avoid CI flakiness or secret requirements.

Recommendation

APPROVE — The fix is correct and minimal: normalizing "." to "" avoids the Gitea API 500 and aligns with the intended semantics of listing the repo root. The added unit test exercises the new behavior and the rest of the code remains unchanged. CI has passed. Optionally, you could extend the normalization to handle "./" or "/" inputs and reconsider the necessity of the extra CI job that depends on external secrets. Otherwise, this is good to merge.


Review by dot-path-test


Evaluated against b0349a22

# Dot-path-test Review ## Summary Change cleanly normalizes a dot path to the repository root in ListContents and adds a focused unit test. CI passed and the implementation is small, safe, and consistent with existing patterns. ## Findings | # | Severity | File | Line | Finding | |---|----------|------|------|--------| | 1 | [MINOR] | `gitea/client.go` | 435 | Consider also normalizing equivalent root path variants such as "./" or "/" (if ever passed) to "" for robustness. Currently only exact "." is handled. | | 2 | [NIT] | `.gitea/workflows/ci.yml` | 69 | The new test-dot-path job introduces an external integration check that depends on secrets and a live repo. Since a unit test already covers the behavior, consider whether this job is necessary to avoid CI flakiness or secret requirements. | ## Recommendation **APPROVE** — The fix is correct and minimal: normalizing "." to "" avoids the Gitea API 500 and aligns with the intended semantics of listing the repo root. The added unit test exercises the new behavior and the rest of the code remains unchanged. CI has passed. Optionally, you could extend the normalization to handle "./" or "/" inputs and reconsider the necessity of the extra CI job that depends on external secrets. Otherwise, this is good to merge. --- *Review by dot-path-test* <!-- review-bot:dot-path-test --> --- *Evaluated against b0349a22*
@@ -66,2 +66,4 @@
SYSTEM_PROMPT_FILE: ${{ matrix.system_prompt_file }}
run: ./review-bot
# Test dot path normalization with security-patterns repo
First-time contributor

[NIT] The new test-dot-path job introduces an external integration check that depends on secrets and a live repo. Since a unit test already covers the behavior, consider whether this job is necessary to avoid CI flakiness or secret requirements.

**[NIT]** The new test-dot-path job introduces an external integration check that depends on secrets and a live repo. Since a unit test already covers the behavior, consider whether this job is necessary to avoid CI flakiness or secret requirements.
@@ -435,6 +435,10 @@ type ContentEntry struct {
// ListContents lists files and directories at a given path in a repo.
First-time contributor

[MINOR] Consider also normalizing equivalent root path variants such as "./" or "/" (if ever passed) to "" for robustness. Currently only exact "." is handled.

**[MINOR]** Consider also normalizing equivalent root path variants such as "./" or "/" (if ever passed) to "" for robustness. Currently only exact "." is handled.
security-review-bot requested review from security-review-bot 2026-05-11 14:12:23 +00:00
security-review-bot approved these changes 2026-05-11 14:12:23 +00:00
Dismissed
security-review-bot left a comment
Collaborator

Original review

Supersededsee current review for up-to-date findings.

Previous findings (commit b0349a22)

Security Review

Summary

The change cleanly normalizes a '.' path to the repository root in ListContents and adds a focused test. CI is passing and there are no new security vulnerabilities in the application code.

Findings

# Severity File Line Finding
1 [MINOR] .gitea/workflows/ci.yml 65 A new pull_request job ('test-dot-path') runs code from the PR with repository secrets (e.g., GPT_REVIEW_TOKEN, AICORE credentials) in the environment. If PRs from forks can trigger this workflow with secrets, a malicious change could exfiltrate them. Ensure secrets are not exposed to untrusted forks or gate this job to trusted actors only.

Recommendation

APPROVE — The code change in gitea/client.go is correct and safe: normalizing a single '.' path to the empty string avoids Gitea API 500 responses without introducing injection or traversal risks. The added test validates the behavior. CI is green.

One minor security consideration: the newly added CI job ('test-dot-path') passes secrets to a binary built from the PR. Confirm that your CI is configured to withhold secrets for untrusted/forked PRs or restrict this job to trusted contributors/branches. Otherwise, consider using a less-privileged token for this test or running it without secrets.

No other security issues were introduced by this PR.


Review by security


Evaluated against b0349a22

~~Original review~~ **Superseded** — [see current review](https://gitea.weiker.me/rodin/review-bot/pulls/72#pullrequestreview-2529) for up-to-date findings. <details><summary>Previous findings (commit b0349a22)</summary> # Security Review ## Summary The change cleanly normalizes a '.' path to the repository root in ListContents and adds a focused test. CI is passing and there are no new security vulnerabilities in the application code. ## Findings | # | Severity | File | Line | Finding | |---|----------|------|------|--------| | 1 | [MINOR] | `.gitea/workflows/ci.yml` | 65 | A new pull_request job ('test-dot-path') runs code from the PR with repository secrets (e.g., GPT_REVIEW_TOKEN, AICORE credentials) in the environment. If PRs from forks can trigger this workflow with secrets, a malicious change could exfiltrate them. Ensure secrets are not exposed to untrusted forks or gate this job to trusted actors only. | ## Recommendation **APPROVE** — The code change in gitea/client.go is correct and safe: normalizing a single '.' path to the empty string avoids Gitea API 500 responses without introducing injection or traversal risks. The added test validates the behavior. CI is green. One minor security consideration: the newly added CI job ('test-dot-path') passes secrets to a binary built from the PR. Confirm that your CI is configured to withhold secrets for untrusted/forked PRs or restrict this job to trusted contributors/branches. Otherwise, consider using a less-privileged token for this test or running it without secrets. No other security issues were introduced by this PR. --- *Review by security* <!-- review-bot:security --> --- *Evaluated against b0349a22* </details> <!-- review-bot:security -->
@@ -65,3 +65,34 @@ jobs:
LLM_TIMEOUT: "600"
Collaborator

[MINOR] A new pull_request job ('test-dot-path') runs code from the PR with repository secrets (e.g., GPT_REVIEW_TOKEN, AICORE credentials) in the environment. If PRs from forks can trigger this workflow with secrets, a malicious change could exfiltrate them. Ensure secrets are not exposed to untrusted forks or gate this job to trusted actors only.

**[MINOR]** A new pull_request job ('test-dot-path') runs code from the PR with repository secrets (e.g., GPT_REVIEW_TOKEN, AICORE credentials) in the environment. If PRs from forks can trigger this workflow with secrets, a malicious change could exfiltrate them. Ensure secrets are not exposed to untrusted forks or gate this job to trusted actors only.
security-review-bot marked this conversation as resolved
rodin added 1 commit 2026-05-11 14:12:32 +00:00
ci: use rodin/security-patterns with '.' path for security reviewer
PR Ready Gate / clear-labels (pull_request) Successful in 2s
CI / test (pull_request) Successful in 17s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 23s
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 46s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 52s
ea74f7e088
Tests the dot path normalization fix end-to-end.
rodin force-pushed issue-70 from b0349a22a0 to ea74f7e088 2026-05-11 14:12:32 +00:00 Compare
sonnet-review-bot approved these changes 2026-05-11 14:13:13 +00:00
sonnet-review-bot left a comment
First-time contributor

Sonnet Review

Summary

Small, well-targeted fix that normalizes '.' to empty string at the start of ListContents() to prevent Gitea API 500 errors. The change is correct, minimal, well-commented, and accompanied by a focused test. The CI workflow change correctly makes patterns_repo and patterns_files matrix-configurable.

Recommendation

APPROVE — Approve. The fix is exactly right: the normalization happens at the single entry point (ListContents), the comment explains the why, the test directly verifies the expected URL is hit, and all existing tests continue to pass per CI. No issues found.


Review by sonnet


Evaluated against ea74f7e0

# Sonnet Review ## Summary Small, well-targeted fix that normalizes '.' to empty string at the start of ListContents() to prevent Gitea API 500 errors. The change is correct, minimal, well-commented, and accompanied by a focused test. The CI workflow change correctly makes patterns_repo and patterns_files matrix-configurable. ## Recommendation **APPROVE** — Approve. The fix is exactly right: the normalization happens at the single entry point (ListContents), the comment explains the why, the test directly verifies the expected URL is hit, and all existing tests continue to pass per CI. No issues found. --- *Review by sonnet* <!-- review-bot:sonnet --> --- *Evaluated against ea74f7e0*
security-review-bot requested review from security-review-bot 2026-05-11 14:13:37 +00:00
security-review-bot approved these changes 2026-05-11 14:13:37 +00:00
security-review-bot left a comment
Collaborator

Security Review

Summary

The changes normalize a '.' path to the repository root for Gitea contents listing and adjust CI to use security patterns with matrix-provided defaults. There are no security-relevant risks introduced by these changes.

Recommendation

APPROVE — Approve the PR. The normalization of '.' to an empty path in ListContents prevents a server-side error without affecting security, and the CI workflow modifications use static matrix values and safe defaults. No injection, auth/authorization, secrets exposure, or other security concerns are introduced.


Review by security


Evaluated against ea74f7e0

# Security Review ## Summary The changes normalize a '.' path to the repository root for Gitea contents listing and adjust CI to use security patterns with matrix-provided defaults. There are no security-relevant risks introduced by these changes. ## Recommendation **APPROVE** — Approve the PR. The normalization of '.' to an empty path in ListContents prevents a server-side error without affecting security, and the CI workflow modifications use static matrix values and safe defaults. No injection, auth/authorization, secrets exposure, or other security concerns are introduced. --- *Review by security* <!-- review-bot:security --> --- *Evaluated against ea74f7e0*
gpt-review-bot approved these changes 2026-05-11 14:13:43 +00:00
gpt-review-bot left a comment
First-time contributor

Gpt Review

Summary

Change correctly normalizes "." to root for Gitea contents API and adds a focused test. CI is green and the workflow updates look sound with sensible matrix defaults.

Findings

# Severity File Line Finding
1 [MINOR] gitea/client.go 436 Normalization only handles the exact string ".". Consider also normalizing common root equivalents like "./" and trailing slash variants (e.g., "/" if ever passed) to improve robustness.
2 [NIT] gitea/client.go 435 The ListContents doc comment says "Pass an empty path to list the repository root." Since "." is now accepted, consider updating the comment to note that "." is treated as root for clarity.

Recommendation

APPROVE — The implementation precisely addresses the Gitea 500 error by mapping "." to the root endpoint and includes a targeted test that verifies the request path. Workflow adjustments introduce matrix-configurable defaults for patterns without breaking existing jobs. Consider extending the normalization to also treat "./" (and potentially other benign root forms) as empty, and update the ListContents comment to reflect that "." is accepted as root. Otherwise, the changes are correct and idiomatic.


Review by gpt


Evaluated against ea74f7e0

# Gpt Review ## Summary Change correctly normalizes "." to root for Gitea contents API and adds a focused test. CI is green and the workflow updates look sound with sensible matrix defaults. ## Findings | # | Severity | File | Line | Finding | |---|----------|------|------|--------| | 1 | [MINOR] | `gitea/client.go` | 436 | Normalization only handles the exact string ".". Consider also normalizing common root equivalents like "./" and trailing slash variants (e.g., "/" if ever passed) to improve robustness. | | 2 | [NIT] | `gitea/client.go` | 435 | The ListContents doc comment says "Pass an empty path to list the repository root." Since "." is now accepted, consider updating the comment to note that "." is treated as root for clarity. | ## Recommendation **APPROVE** — The implementation precisely addresses the Gitea 500 error by mapping "." to the root endpoint and includes a targeted test that verifies the request path. Workflow adjustments introduce matrix-configurable defaults for patterns without breaking existing jobs. Consider extending the normalization to also treat "./" (and potentially other benign root forms) as empty, and update the ListContents comment to reflect that "." is accepted as root. Otherwise, the changes are correct and idiomatic. --- *Review by gpt* <!-- review-bot:gpt --> --- *Evaluated against ea74f7e0*
@@ -435,6 +435,10 @@ type ContentEntry struct {
// ListContents lists files and directories at a given path in a repo.
First-time contributor

[NIT] The ListContents doc comment says "Pass an empty path to list the repository root." Since "." is now accepted, consider updating the comment to note that "." is treated as root for clarity.

**[NIT]** The ListContents doc comment says "Pass an empty path to list the repository root." Since "." is now accepted, consider updating the comment to note that "." is treated as root for clarity.
@@ -435,6 +435,10 @@ type ContentEntry struct {
// ListContents lists files and directories at a given path in a repo.
// Pass an empty path to list the repository root.
First-time contributor

[MINOR] Normalization only handles the exact string ".". Consider also normalizing common root equivalents like "./" and trailing slash variants (e.g., "/" if ever passed) to improve robustness.

**[MINOR]** Normalization only handles the exact string ".". Consider also normalizing common root equivalents like "./" and trailing slash variants (e.g., "/" if ever passed) to improve robustness.
aweiker approved these changes 2026-05-11 14:15:38 +00:00
aweiker merged commit 036e96d9b7 into main 2026-05-11 14:16:22 +00:00
Sign in to join this conversation.
5 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: rodin/review-bot#72