From 02dfc12141628ec7bf823048e2b4dcad49417607 Mon Sep 17 00:00:00 2001 From: Rodin Date: Fri, 15 May 2026 11:28:21 +0000 Subject: [PATCH] fix(#143): skip local doc-map validation when --doc-map-trusted-ref is set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When --doc-map-trusted-ref is provided, the --doc-map value is used as a VCS API path parameter, not a local filesystem path. The early call to validateWorkspacePath (which requires the file to exist locally) blocked the trusted-ref code path when the doc-map did not exist in the local checkout — defeating the feature's purpose in sparse checkouts or when the file is only on the default branch. Fix: guard the early validation with `&& *docMapTrustedRef == ""`. Also fixes: - review/docmap.go: correct ParseDocMapConfigContent godoc example to match actual source format "owner/repo@ref:path" - cmd/review-bot/main_test.go: add TestMainSubprocess_DocMapTrustedRefSkipsLocalValidation to prevent regression --- cmd/review-bot/main.go | 7 ++++-- cmd/review-bot/main_test.go | 44 +++++++++++++++++++++++++++++++++++++ review/docmap.go | 2 +- 3 files changed, 50 insertions(+), 3 deletions(-) diff --git a/cmd/review-bot/main.go b/cmd/review-bot/main.go index de489cf..9f9fbb5 100644 --- a/cmd/review-bot/main.go +++ b/cmd/review-bot/main.go @@ -174,9 +174,12 @@ func main() { os.Exit(1) } - // Early validation of filesystem-path flags (fail fast before network I/O) + // Early validation of filesystem-path flags (fail fast before network I/O). + // Skip local-path validation when --doc-map-trusted-ref is set: the flag + // value is used as a VCS API path, not a local filesystem path, and the + // file may not exist in the local checkout (sparse, PR-deleted, etc.). var resolvedDocMapFile string - if *docMapFile != "" { + if *docMapFile != "" && *docMapTrustedRef == "" { resolved, err := validateWorkspacePath(*docMapFile, "doc-map") if err != nil { slog.Error("invalid doc-map path", "error", err) diff --git a/cmd/review-bot/main_test.go b/cmd/review-bot/main_test.go index e9d32c8..db6d536 100644 --- a/cmd/review-bot/main_test.go +++ b/cmd/review-bot/main_test.go @@ -1578,3 +1578,47 @@ func TestMainSubprocess_InvalidDocMapFile(t *testing.T) { t.Errorf("expected error about failed resolution, got: %s", output) } } + +// TestMainSubprocess_DocMapTrustedRefSkipsLocalValidation confirms that +// --doc-map-trusted-ref bypasses local filesystem validation for --doc-map. +// When the trusted-ref flag is set, the doc-map value is used as a VCS API +// path; a nonexistent local file must not cause an early exit before network I/O. +func TestMainSubprocess_DocMapTrustedRefSkipsLocalValidation(t *testing.T) { + if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" { + flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError) + os.Args = []string{"review-bot", + "--vcs-url", "https://gitea.example.com", + "--repo", "owner/repo", + "--pr", "1", + "--reviewer-token", "tok", + "--llm-base-url", "https://api.example.com", + "--llm-api-key", "key", + "--llm-model", "gpt-4", + "--doc-map", "nonexistent-local.yml", + "--doc-map-trusted-ref", "main", + } + main() + return + } + + cmd := exec.Command(os.Args[0], "-test.run=TestMainSubprocess_DocMapTrustedRefSkipsLocalValidation") + cmd.Env = append(cleanEnv(), + "TEST_SUBPROCESS_MAIN=1", + "GITHUB_WORKSPACE="+t.TempDir(), + ) + out, err := cmd.CombinedOutput() + output := string(out) + + // The test must fail (network I/O or VCS API failure) but must NOT + // fail with the local filesystem validation error. + // "failed to resolve" would indicate the early validateWorkspacePath ran — + // that would be the bug this test is catching. + if strings.Contains(output, "failed to resolve") { + t.Errorf("--doc-map-trusted-ref should skip local path validation, but got filesystem error: %s", output) + } + + // It must still exit non-zero (real VCS call to example.com will fail). + if err == nil { + t.Fatal("expected non-zero exit when VCS API is unreachable, got success") + } +} diff --git a/review/docmap.go b/review/docmap.go index 6478ab1..1789e63 100644 --- a/review/docmap.go +++ b/review/docmap.go @@ -57,7 +57,7 @@ func ParseDocMapConfig(localPath string) (*DocMapConfig, error) { // ParseDocMapConfigContent parses a doc-map YAML config from an in-memory // string. The source parameter is used only for error messages and log entries -// (e.g. "main:main@"). +// (e.g. "owner/repo@main:.review-bot/doc-map.yml"). // // Use this when the config content has been fetched from a trusted VCS ref // rather than read from the local workspace.