review-bot/PLAN-125.md

Plan: Issue #125 — Rename GITEA_URL → VCS_URL

Problem

The GITEA_URL environment variable (and --gitea-url flag) implies the binary only works with Gitea. Now that review-bot supports both Gitea and GitHub/GHES, this name is misleading. Renaming to VCS_URL makes the binary platform-agnostic in its interface.

Constraints

• Must not break existing users who already use GITEA_URL — need a fallback
• The CLI flag --gitea-url should also be updated to --vcs-url for consistency
• INTEGRATION_GITEA_URL in integration tests is a test-only env var, not the binary's interface; but should be updated for clarity
• The action YAML uses GITEA_URL as an internal shell variable in bash scripts — distinct from the env var passed to the binary
• All changes must compile and pass existing tests

Files Affected

Binary / Go source

File	Change
cmd/review-bot/main.go	Rename --gitea-url → --vcs-url, add VCS_URL as primary, keep GITEA_URL fallback
cmd/review-bot/integration_test.go	Rename INTEGRATION_GITEA_URL → INTEGRATION_VCS_URL (test-only, no external compat concern)
integration_test.go	Same — rename INTEGRATION_GITEA_URL → INTEGRATION_VCS_URL

Action YAML

File	Change
.gitea/actions/review/action.yml	Rename input gitea-url → vcs-url; update env var passed to binary: VCS_URL instead of GITEA_URL; keep internal bash var as GITEA_URL (only used for release download, not passed to binary)
.gitea/workflows/ci.yml	Rename GITEA_URL env var to VCS_URL in Run review step

Documentation

File	Change
README.md	Update CLI example, env var table entry

Proposed Approach

1. Backward-compatible env var lookup in main.go

Replace:

giteaURL := flag.String("gitea-url", envOrDefault("GITEA_URL", ""), "Gitea instance URL")

With:

giteaURL := flag.String("vcs-url", envOrDefaultFallback("VCS_URL", "GITEA_URL", ""), "VCS server URL (e.g. https://gitea.example.com)")

Add a helper:

// envOrDefaultFallback reads primary env var; if empty, falls back to deprecated env var.
func envOrDefaultFallback(primary, deprecated, defaultVal string) string {
    if v := os.Getenv(primary); v != "" {
        return v
    }
    if v := os.Getenv(deprecated); v != "" {
        slog.Warn("deprecated env var in use; rename to " + primary, "old", deprecated, "new", primary)
        return v
    }
    return defaultVal
}

Note: This must be called AFTER setupLogger conceptually, but the flag default is evaluated at flag registration time. Since setupLogger runs before flag.Parse(), the slog.Warn will print correctly at runtime. We use log.Printf as a fallback if this proves problematic.

Actually — flag defaults are evaluated at registration (line 57), before setupLogger. The warning won't go through slog. Two options:

• Use log.Printf for the deprecation warning (always visible)
• Move the fallback lookup to after flag.Parse(), checking if the parsed value is still empty

Decision: Move fallback to a post-parse check. This is cleaner:

vcsURL := flag.String("vcs-url", os.Getenv("VCS_URL"), "VCS server URL")
flag.Parse()
// Backward compat: fall back to deprecated GITEA_URL
if *vcsURL == "" {
    if v := os.Getenv("GITEA_URL"); v != "" {
        slog.Warn("GITEA_URL is deprecated; use VCS_URL instead")
        *vcsURL = v
    }
}

This is clean, idiomatic, and the warning goes through slog correctly.

2. Keep --gitea-url as deprecated alias

Add a hidden flag for backward compat:

giteaURLAlias := flag.String("gitea-url", "", "Deprecated: use --vcs-url")

Post-parse:

if *vcsURL == "" && *giteaURLAlias != "" {
    slog.Warn("--gitea-url is deprecated; use --vcs-url instead")
    *vcsURL = *giteaURLAlias
}

3. Internal variable rename

Rename giteaURL local variable → vcsURL throughout main.go for consistency.

4. Error message update

fmt.Fprintf(os.Stderr, "Required: --vcs-url, --repo, --pr, --reviewer-token, --llm-model\n")

5. Action YAML changes

In .gitea/actions/review/action.yml:

• Input gitea-url → vcs-url (with same description, required: false, default: '')
• Line 172: GITEA_URL: ${{ inputs.gitea-url || github.server_url }} → VCS_URL: ${{ inputs.vcs-url || github.server_url }}
• Lines 115, 140: internal bash vars GITEA_URL= are used for downloading binaries — NOT passed to the review-bot binary. Leave them as internal bash vars (they're scope-local in bash). These could be renamed to SERVER_URL or BASE_URL for local clarity, but renaming them isn't strictly required.

In .gitea/workflows/ci.yml:

• Line 52: GITEA_URL: ${{ github.server_url }} → VCS_URL: ${{ github.server_url }}

6. Integration test updates

INTEGRATION_GITEA_URL → INTEGRATION_VCS_URL in both test files.

7. README

• CLI example: --gitea-url → --vcs-url
• Env var table: GITEA_URL → VCS_URL, add note about GITEA_URL fallback

Backward Compatibility Summary

Old	New	Fallback?
GITEA_URL env var	VCS_URL	✅ with deprecation warning
--gitea-url flag	--vcs-url	✅ with deprecation warning
gitea-url action input	vcs-url	⚠️ No (action version bump handles this)
INTEGRATION_GITEA_URL	INTEGRATION_VCS_URL	N/A (test-only)

Error Cases

• Both VCS_URL and GITEA_URL set: VCS_URL wins (primary takes precedence)
• Both --vcs-url and --gitea-url provided: --vcs-url wins
• Neither set: existing "missing required flags" error unchanged

Edge Cases

• os.Getenv returns "" for unset AND set-to-empty — consistent with existing behavior
• The envOrDefault helper is unchanged; we add envOrDefaultFallback for the one renamed var

Testing Strategy

• Existing unit tests pass unchanged (they don't test env var parsing directly)
• Integration tests updated to use new env var name
• Manual: GITEA_URL=https://example.com ./review-bot --repo x --pr 1 ... should print deprecation warning and proceed
• Manual: VCS_URL=https://example.com ./review-bot ... should work silently

Completion Checklist

1. VCS_URL is read first; GITEA_URL is fallback with deprecation warning
2. --vcs-url flag is primary; --gitea-url is deprecated alias with warning
3. Error message references --vcs-url not --gitea-url
4. action.yml passes VCS_URL (not GITEA_URL) to the binary
5. ci.yml passes VCS_URL (not GITEA_URL) to the binary
6. README updated in CLI example and env var table
7. Integration tests use INTEGRATION_VCS_URL
8. go test ./... passes
9. go vet ./... passes
10. go build ./cmd/review-bot succeeds

Open Questions

• Should the CLI flag --gitea-url be completely hidden from --help or just deprecated with a note? The issue doesn't specify. Decision: keep it visible but add "(deprecated: use --vcs-url)" to the description.
• Should action.yml also add gitea-url as a deprecated input alias? The issue says "Update the action to pass the new env var name" — no mention of backward compat for the action input. Decision: rename only, no alias (action users pin a version anyway).
• The bash-internal GITEA_URL variable in action.yml scripts (used for release download, not passed to binary) — rename for clarity? Decision: yes, rename to BASE_URL to avoid confusion with the env var.