Apply reviewed labels without risking PR creation

backend/app/routes/github.py

@@ -1032,6 +1032,153 @@ def _parse_pr_number(url: str) -> int | None:
   return int(m.group(1)) if m else None
 
 
+def _reviewed_pr_labels(plan: dict) -> list[str]:
+  """Return only the two labels the owner could see in Contribute review."""
+  raw = plan.get("labels")
+  if not isinstance(raw, list):
+    return []
+  # Mirror Contribute's review surface: it filters malformed/blank values,
+  # trims them, and then shows at most two. Security validation and duplicate
+  # folding happen only after that visibility boundary, so an unseen third
+  # label can never replace a visible-but-unusable one at submit time.
+  visible = []
+  for value in raw:
+    if not isinstance(value, str):
+      continue
+    label = value.strip()
+    if not label:
+      continue
+    visible.append(label)
+    if len(visible) == 2:
+      break
+  labels = []
+  seen = set()
+  for label in visible:
+    folded = label.casefold()
+    if len(label) > 50 or "\n" in label or folded in seen:
+      continue
+    seen.add(folded)
+    labels.append(label)
+  return labels
+
+
+def _apply_reviewed_pr_labels(
+  repo: Path,
+  upstream_repo: str,
+  number: int | None,
+  labels: list[str],
+) -> dict:
+  """Best-effort add reviewed labels that already exist in the target repo.
+
+  Labeling is deliberately secondary to PR creation: a missing repository
+  label, permission restriction, or transient API failure must not turn an
+  already-open pull request into an apparent failed submission. The outcome is
+  persisted so the review never claims an unavailable label was applied.
+  """
+  if not labels:
+    return {}
+  patch = {
+    "last_submit_labels_requested": labels,
+    "last_submit_labels_applied": [],
+  }
+  if number is None:
+    return {
+      **patch,
+      "last_submit_labels_note": "GitHub did not return a PR number for labeling.",
+    }
+
+  try:
+    available = _gh(
+      repo,
+      "api", "--paginate",
+      f"repos/{upstream_repo}/labels?per_page=100",
+      "--jq", ".[].name",
+      check=False,
+    )
+  except subprocess.TimeoutExpired:
+    return {
+      **patch,
+      "last_submit_labels_note": (
+        "Timed out while checking repository labels; the pull request is "
+        "open without confirmed labels."
+      ),
+    }
+  except OSError:
+    return {
+      **patch,
+      "last_submit_labels_note": (
+        "Could not start the GitHub label lookup; the pull request is open "
+        "without confirmed labels."
+      ),
+    }
+  if available.returncode != 0:
+    return {
+      **patch,
+      "last_submit_labels_note": (
+        "Could not verify the repository labels; the pull request is open "
+        "without confirmed labels."
+      ),
+    }
+  by_name = {}
+  for raw_name in (available.stdout or "").splitlines():
+    name = raw_name.strip()
+    if name:
+      by_name[name.casefold()] = name
+  applicable = [by_name[label.casefold()] for label in labels
+                if label.casefold() in by_name]
+  missing = [label for label in labels if label.casefold() not in by_name]
+  if not applicable:
+    return {
+      **patch,
+      "last_submit_labels_missing": missing,
+      "last_submit_labels_note": "The reviewed labels do not exist in this repository.",
+    }
+
+  try:
+    applied = _gh(
+      repo,
+      "api", "--method", "POST",
+      f"repos/{upstream_repo}/issues/{number}/labels",
+      *(part for label in applicable for part in ("-f", f"labels[]={label}")),
+      check=False,
+    )
+  except subprocess.TimeoutExpired:
+    return {
+      **patch,
+      "last_submit_labels_missing": missing,
+      "last_submit_labels_note": (
+        "Timed out while applying reviewed labels; the pull request is open, "
+        "but GitHub did not confirm the label result."
+      ),
+    }
+  except OSError:
+    return {
+      **patch,
+      "last_submit_labels_missing": missing,
+      "last_submit_labels_note": (
+        "Could not start the GitHub label update; the pull request is open "
+        "without confirmed labels."
+      ),
+    }
+  if applied.returncode != 0:
+    return {
+      **patch,
+      "last_submit_labels_missing": missing,
+      "last_submit_labels_note": (
+        "GitHub did not confirm these labels were applied; the pull request "
+        "is still open."
+      ),
+    }
+  result = {
+    **patch,
+    "last_submit_labels_applied": applicable,
+  }
+  if missing:
+    result["last_submit_labels_missing"] = missing
+    result["last_submit_labels_note"] = "Some reviewed labels no longer exist."
+  return result
+
+
 def _find_existing_pr(
   repo: Path,
   upstream_repo: str,
@@ -1702,7 +1849,18 @@ def _submit_prepared_pr(
             same_repo=bool(direct_base),
           )
           if existing:
-            return existing, _parse_pr_number(existing), pushed_patch
+            existing_number = _parse_pr_number(existing)
+            label_patch = _apply_reviewed_pr_labels(
+              repo,
+              upstream_repo,
+              existing_number,
+              _reviewed_pr_labels(plan),
+            )
+            return (
+              existing,
+              existing_number,
+              _record_patch_with(pushed_patch, label_patch),
+            )
           detail = (pr.stderr or pr.stdout or "GitHub command failed.").strip()
           raise ContributionSubmitError(detail[:600] or "GitHub command failed.")
       except ContributionSubmitError as exc:
@@ -1723,7 +1881,14 @@ def _submit_prepared_pr(
         f"to {pushed_branch_url}.",
         record_patch=pushed_patch,
       )
-    return url, _parse_pr_number(url), pushed_patch
+    number = _parse_pr_number(url)
+    label_patch = _apply_reviewed_pr_labels(
+      repo,
+      upstream_repo,
+      number,
+      _reviewed_pr_labels(plan),
+    )
+    return url, number, _record_patch_with(pushed_patch, label_patch)
   finally:
     if checkout_back:
       _git(repo, "checkout", "-q", checkout_back, check=False)

backend/tests/test_github_routes.py

@@ -748,6 +748,97 @@ def test_graphql_mutation_as_string_literal_allowed(client, auth, monkeypatch):
 # --- contribution submit (approval button path) -----------------------
 
 
+def test_reviewed_pr_labels_are_bounded_to_the_visible_two():
+  assert github_routes._reviewed_pr_labels({
+    "labels": [" bug ", "area: ui", "hidden-third"],
+  }) == ["bug", "area: ui"]
+  assert github_routes._reviewed_pr_labels({
+    "labels": ["bug", "BUG", "area: ui"],
+  }) == ["bug"]
+  assert github_routes._reviewed_pr_labels({
+    "labels": [None, "", "bug", "area: ui", "hidden-third"],
+  }) == ["bug", "area: ui"]
+  assert github_routes._reviewed_pr_labels({"labels": "bug"}) == []
+
+
+def test_pr_labels_apply_only_existing_names_and_preserve_missing(
+  monkeypatch, tmp_path,
+):
+  calls = []
+
+  def fake_gh(repo, *args, check=True):
+    calls.append(args)
+    if "--paginate" in args:
+      return _cp("bug\narea: ui\n")
+    return _cp("[]")
+
+  monkeypatch.setattr(github_routes, "_gh", fake_gh)
+  patch = github_routes._apply_reviewed_pr_labels(
+    tmp_path,
+    "mobius-os/mobius",
+    123,
+    ["Bug", "area: backend"],
+  )
+
+  assert patch["last_submit_labels_requested"] == ["Bug", "area: backend"]
+  assert patch["last_submit_labels_applied"] == ["bug"]
+  assert patch["last_submit_labels_missing"] == ["area: backend"]
+  assert "Some reviewed labels" in patch["last_submit_labels_note"]
+  apply_call = calls[-1]
+  assert apply_call[:3] == ("api", "--method", "POST")
+  assert "labels[]=bug" in apply_call
+  assert "labels[]=area: backend" not in apply_call
+
+
+def test_pr_label_permission_failure_does_not_fail_an_open_pr(
+  monkeypatch, tmp_path,
+):
+  def fake_gh(repo, *args, check=True):
+    if "--paginate" in args:
+      return _cp("bug\n")
+    return _cp("forbidden", returncode=1)
+
+  monkeypatch.setattr(github_routes, "_gh", fake_gh)
+  patch = github_routes._apply_reviewed_pr_labels(
+    tmp_path,
+    "someone/example",
+    7,
+    ["bug"],
+  )
+
+  assert patch["last_submit_labels_applied"] == []
+  assert "did not confirm" in patch["last_submit_labels_note"]
+
+
+@pytest.mark.parametrize(
+  "label_failure",
+  [
+    subprocess.TimeoutExpired(["gh", "api"], timeout=30),
+    OSError("gh could not start"),
+  ],
+  ids=["apply-timeout", "apply-launch-error"],
+)
+def test_pr_label_apply_transport_failure_is_nonfatal(
+  monkeypatch, tmp_path, label_failure,
+):
+  def fake_gh(repo, *args, check=True):
+    if "--paginate" in args:
+      return _cp("bug\n")
+    raise label_failure
+
+  monkeypatch.setattr(github_routes, "_gh", fake_gh)
+  patch = github_routes._apply_reviewed_pr_labels(
+    tmp_path,
+    "someone/example",
+    7,
+    ["bug"],
+  )
+
+  assert patch["last_submit_labels_requested"] == ["bug"]
+  assert patch["last_submit_labels_applied"] == []
+  assert "pull request is open" in patch["last_submit_labels_note"]
+
+
 def _write_contribution(app_id, record_id, record, diff_text=""):
   base = Path(get_settings().data_dir) / "apps" / str(app_id) / "contributions"
   base.mkdir(parents=True, exist_ok=True)
@@ -1708,12 +1799,21 @@ def _commit_metadata(
   )
 
 
-def test_submit_contribution_creates_review_ready_pr_from_prepared_record(
-  client, owner_token, monkeypatch,
+@pytest.mark.parametrize(
+  "failure_kind",
+  ["timeout", "launch-error"],
+)
+def test_submit_contribution_keeps_accepted_pr_open_on_label_transport_failure(
+  client, owner_token, monkeypatch, failure_kind,
 ):
+  label_failure = (
+    subprocess.TimeoutExpired(["gh", "api"], timeout=30)
+    if failure_kind == "timeout"
+    else OSError("gh could not start")
+  )
   _write_token(login="octocat")
   app_id, app_token = _app_token(client, owner_token, github_access=True)
-  record_id = "rec-pr-1"
+  record_id = f"rec-pr-label-{failure_kind}"
   repo = Path(get_settings().data_dir) / "contributions" / record_id / "repo"
   (repo / ".git").mkdir(parents=True)
   diff_text = "diff --git a/index.jsx b/index.jsx\n+hello\n"
@@ -1738,6 +1838,7 @@ def test_submit_contribution_creates_review_ready_pr_from_prepared_record(
       "base_sha": base,
       "head_sha": head,
       "diff_sha256": hashlib.sha256(diff_text.encode()).hexdigest(),
+      "labels": ["bug"],
     },
   }
   _write_contribution(app_id, record_id, record, diff_text)
@@ -1803,6 +1904,8 @@ def fake_gh(repo_path, *args, check=True):
       return _cp("[]")
     if args[:2] == ("pr", "create"):
       return _cp("https://github.com/mobius-os/app-demo/pull/42\n")
+    if args[:2] == ("api", "--paginate"):
+      raise label_failure
     return _cp("")
 
   monkeypatch.setattr("app.routes.github._git", fake_git)
@@ -1818,6 +1921,9 @@ def fake_gh(repo_path, *args, check=True):
   assert body["number"] == 42
   assert body["record"]["status"] == "open"
   assert body["record"]["url"] == body["url"]
+  assert body["record"]["last_submit_labels_requested"] == ["bug"]
+  assert body["record"]["last_submit_labels_applied"] == []
+  assert "pull request is open" in body["record"]["last_submit_labels_note"]
   assert ("repo", "fork", "--remote", "--remote-name", "fork") in gh_calls
   assert not any(call[:2] == ("remote", "set-url") for call in git_calls)
   create_call = next(call for call in gh_calls if call[:2] == ("pr", "create"))
@@ -1837,6 +1943,9 @@ def fake_gh(repo_path, *args, check=True):
   assert stored["status"] == "open"
   assert stored["number"] == 42
   assert stored["head_repository"] == "octocat/app-demo-1"
+  assert stored["last_submit_labels_requested"] == ["bug"]
+  assert stored["last_submit_labels_applied"] == []
+  assert stored["last_submit_labels_note"] == body["record"]["last_submit_labels_note"]
 
 
 def test_submit_contribution_normalizes_fallback_author_before_push(