diff --git a/CHANGELOG.md b/CHANGELOG.md index 1da7a46d..c705ac02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -205,6 +205,14 @@ per-file diff commands. - **`freehire-search` rejects an unknown `--remote` mode instead of searching a facet that matches nothing** (#661, `.agents/skills/freehire-search/cli/src/cli.ts`, `cli/tests/remote-mode.test.ts`) - `--remote foo` was forwarded as `work_mode=foo` and the CLI exited 0 with an empty result set, so a typo looked like "no jobs". The documented `remote` / `hybrid` / `onsite` values, mixed case, and `on-site` now map to the facet; a bare `--remote` still means `remote`. Comma-separated modes are validated per item and re-joined (`remote,hybrid`, `remote,on-site` → `remote,onsite`); an unknown item such as `foo` in `remote,foo` exits 1 with `BAD_ARG` and does not call the API. +- **`check_upstream_updates.py` no longer reports "up to date" after a failed fetch** (#681, + `tools/check_upstream_updates.py`, `tests/test_check_upstream_updates.py`) - when fetching upstream + failed but a cached `upstream/master` existed, the checker warned about the fetch, compared against + the stale cache and could still end with `[OK] All framework files are up to date`, an all-clear it + could not establish. The cached comparison still runs, but a failed fetch now ends with a + `[WARNING]` that the result used cached data, and never with `[OK]`. A successful fetch and + `--no-fetch` are unchanged. + ### Security - **CI no longer leaves the job token in `.git/config` or expands a PR-controlled name into a diff --git a/tests/test_check_upstream_updates.py b/tests/test_check_upstream_updates.py index e1bed589..d38cc120 100644 --- a/tests/test_check_upstream_updates.py +++ b/tests/test_check_upstream_updates.py @@ -68,6 +68,14 @@ class UpstreamCheckerRepoFixture(unittest.TestCase): text=True, ) + def run_checker_with_fetch(self, *args) -> subprocess.CompletedProcess: + return subprocess.run( + [sys.executable, str(self.root / "tools" / "check_upstream_updates.py"), *args], + cwd=self.root, + capture_output=True, + text=True, + ) + class ForkWithoutUpstreamRemoteTests(UpstreamCheckerRepoFixture): def setUp(self): @@ -132,6 +140,23 @@ class UpstreamRemotePresentTests(UpstreamCheckerRepoFixture): self.assertIn("up to date with upstream/master", result.stdout) +class FailedFetchTests(UpstreamCheckerRepoFixture): + def test_cached_ref_after_failed_fetch_is_not_reported_as_current(self): + # Normal CLI path: a fork has a previous upstream/master ref, but its + # remote is unavailable today. The cached comparison can be useful, + # but it cannot establish whether the fork is currently up to date. + self.add_remote("upstream", str(self.root / "unavailable-remote")) + self.materialize_remote_ref("upstream") + + result = self.run_checker_with_fetch() + + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("Failed to fetch", result.stdout) + self.assertIn("cached", result.stdout.lower()) + self.assertIn("[WARNING]", result.stdout) + self.assertNotIn("[OK]", result.stdout) + + class UpstreamRefMissingFileTests(UpstreamCheckerRepoFixture): """Simulates upstream renaming/deleting one framework file while the fork still has its own copy: git show then fails, and the checker used diff --git a/tools/check_upstream_updates.py b/tools/check_upstream_updates.py index 4dd8c9db..9dcb040a 100755 --- a/tools/check_upstream_updates.py +++ b/tools/check_upstream_updates.py @@ -97,10 +97,12 @@ def main() -> int: f" git remote add upstream https://github.com/{UPSTREAM_REPO_SLUG}.git" ) + fetch_failed = False if not args.no_fetch: print(f"Fetching latest from remote '{remote}'...") rc, _, stderr = run_git(["fetch", remote]) if rc != 0: + fetch_failed = True print(f"Warning: Failed to fetch from remote '{remote}': {stderr.strip()}") print("Proceeding with cached remote tracking branches.") @@ -184,8 +186,13 @@ def main() -> int: f"{len(errors)} configuration error(s), {len(missing_upstream)} file(s) missing upstream. " "Review the messages above before assuming you are up to date." ) - else: + elif not fetch_failed: print(f"[OK] All framework files are up to date with {ref}!") + if fetch_failed: + print( + f"[WARNING] Compared against cached {ref} because the fetch failed; " + "the current upstream state could not be checked." + ) # Version stamps answer "which of my files changed"; commit-level triage # answers "which upstream commits deserve review". Point at the companion. print(