fix(upstream): avoid all-clear after a failed fetch (#681)

* fix(upstream): avoid all-clear after a failed fetch

* changelog: place the #681 entry under [Unreleased]

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Mads Lorentzen <madslorentzen_17@hotmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

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

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

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(