Files
review-bot/TODO.md
T

89 lines
4.2 KiB
Markdown

## Dev Loop: review-bot — 2026-05-14 20:10 UTC
### Latest: ✅ STABLE STATE — REPO HEALTH COMPLETE
- **Last action:** health check; verified tests pass, repo clean, no action needed
- **Repository:** Clean, all merges complete, no open issues/PRs
- **Main branch:** Up to date with origin/main
- **Test suite:** All passing (cached)
---
## Repository Status
### ✅ Merged to main (recent):
- issue-123 (IP-level SSRF defense) — 6 commits, main at 4440823
- issue-125 (VCS_URL rename + deprecation) — merged
- issue-124 (multi-arch binary support) — merged
- issue-120 (GitHub Actions + VCS abstraction) — merged
- issue-121 (VCS host type detection for binary download) — merged
### 🧹 Cleanup COMPLETE:
- ✅ Removed old worktrees (issue-123, review-bot-issue-125)
- ✅ Test suite passes (all packages)
- ✅ No TODO/FIXME in code except expected GitHub client notes
- ✅ No open issues or pull requests
- ✅ Dependencies up to date
---
## Current Feature Completeness
**Core Capabilities:**
- Multi-provider LLM support (OpenAI, Anthropic, SAP AI Core)
- Gitea PR integration with structured reviews
- SSRF defense with IP-level validation
- VCS abstraction (Gitea/GitHub support)
- Multi-architecture binary support
- GitHub Actions composite action
**Recent Security Work:**
- RFC6598 CGN range detection
- IP fallback dialing for local endpoint rejection
- URL validation for SSRF prevention
**Code Quality:**
- Comprehensive test coverage (all packages tested)
- Consistent error handling with context propagation
- Secure credential handling (unexported fields)
- Concurrency-safe designs
---
## Next Priority Actions
### Phase 2: Feature Exploration (NEXT SESSION)
- Scan code for potential improvements per REVIEW.md findings
- Assess performance under load
- Review REVIEW.md findings for targeted fixes
- Consider backlog items from design docs
### Phase 3: Optional Enhancements (BACKLOG)
- Address REVIEW.md context propagation findings (if prioritized)
- Additional LLM provider support
- Enhanced context detection
- Custom report formats
- Webhook management improvements
---
## Worktrees Status
All old worktrees cleaned up. Ready for new issue work.
---
## Dev-Loop Metadata
- **Repo:** /home/ubuntu/review-bot
- **Main branch SHA:** ed3a5dd (last commit)
- **Cron ID:** 5342ac81-4bbc-4e4c-a123-347a7788d50c
- **Scheduled:** Every 4 hours
- **Last health check:** 2026-05-14 20:10 UTC (✅ all healthy)
## Self-Review: review-bot-issue-130-work — 2026-05-14
### Verdict: NEEDS_WORK
- [x] [completeness] `VCS_TYPE` env var is detected in `main.go` but **not passed from `action.yml`'s `Run review` step** to the binary. The binary falls back to a URL heuristic (`github.com` / `github.concur.com` substrings), which will silently misclassify any GitHub Enterprise Server whose hostname does not contain `github`. The action already has `vcs_type` as a step output — it should be passed as `VCS_TYPE: ${{ steps.version.outputs.vcs_type }}` in the `Run review` env block.
- [x] [completeness] `README.md` still references `GITEA_REPO` and `$GITEA_TOKEN` in the CLI example (line ~288) and the env var table (line ~303). Now that the binary supports GitHub too, these should be renamed to VCS-agnostic names (`VCS_REPO`/`REPO`, `VCS_TOKEN`/`REVIEWER_TOKEN`) or at minimum the env var table should note GitHub support.
- [x] [fit] `vcsReviewComment.NewPosition` field comment says "Gitea: absolute line; GitHub: diff hunk position" but in `githubVCSAdapter.PostReview` it is actually mapped to `Line` (absolute line) + `Side: "RIGHT"`, not to `Position` (hunk position). The field name and comment are now slightly misleading — the field means "new line number" for Gitea and is repurposed as "absolute line" for GitHub. Consider renaming to `Line` or `NewLine` with a clearer comment explaining the per-backend semantics.
- [x] [instinct] `validateurl.go` imports `gitea` package for `gitea.IsBlockedIP` — this creates a dependency from the GitHub-generic `validateurl.go` on the Gitea-specific package. The `IsBlockedIP` function is a general networking utility. Consider moving it to a shared `internal/netutil` package (or keeping it in `gitea` and accepting the coupling), but the current import is an unexpected relationship.