ci: run External CI from the manifest.json of the revision under test; real conclusions for notify repos

.github/actions/ext-ci/README.md

@@ -18,20 +18,33 @@ also runs for merge groups and on manual dispatch (`repos`, `pr_number` or `fpri
 | `repos[].workflow` | workflow file in that repository; must accept the inputs below |
 | `repos[].default_ref` | branch to dispatch when neither `pr-<N>` nor a branch named like the F´ base branch exists there |
 | `repos[].timeout_minutes` | remote running time allowed, and separately the time it may wait in queue; the remote run is cancelled on timeout. Keep `2 * timeout_minutes` under the job's `timeout-minutes` in ext-ci.yml |
-| `repos[].failure_policy` | `block`: a failure is a red check and fails the orchestrator job; `notify`: neutral (non-blocking) check |
-| `repos[].on_pr` | `false`: never dispatched for a PR head (merge queue and manual `fprime_ref` runs only) |
+| `repos[].failure_policy` | `block`: a failure is a red check and fails the orchestrator job; `notify`: the check still shows the real result (red on failure) but the orchestrator job stays green; do not make it a required check |
+| `repos[].on_pr` | `false`: never dispatched for a PR head (merge queue and manual `fprime_ref` runs only), except once on the PR that adds the entry (see below) |
 
 Orchestrator-side faults (dispatch error, run never started, polling lost the run) fail the job
 whatever the policy. A re-run of the orchestrator re-dispatches only the repos whose check is not green.
 
+### Which manifest.json is used
+
+The one at the F´ revision under test: the PR head, the merge-queue commit or the manual `fprime_ref`.
+It is read as data through this repository's contents API (PR head commits are reachable here, fork or
+not); the workflow and the script come from the default branch, whose `manifest.json` only serves the
+`upstream_repository` preflight. So a PR that adds or edits an entry exercises it on its own checks, and
+entries the default branch does not have ignore `on_pr`. The external repository must already be in the
+External CI App installation or the check reports `dispatch failed ... 403`. A remote run held by a
+protected environment shows as "Waiting for approval"; both timeout clocks restart when it is released
+and the orchestrator job's `timeout-minutes` bounds the wait.
+
 ## Contract for an external `ci.yml`
 
 - `on: workflow_dispatch` with inputs `fprime_repository`, `fprime_ref` (exact SHA to use as the F´
   dependency), `fprime_pr_number`, `dispatch_id`.
 - `permissions: contents: read` (or `{}`), no `${{ secrets.* }}`, no Actions cache writes on dispatched
   runs: the dispatched run executes untrusted F´ PR code in the external repository's own context.
 - Repositories with self-hosted or hardware runners: `on_pr: false` in the manifest, ephemeral runners,
-  SHA-pinned actions, and a job-level guard on `inputs.fprime_repository` in the workflow.
+  SHA-pinned actions, a protected environment with required reviewers on the hardware job, and a job-level
+  guard on `inputs.fprime_repository` in the workflow. `on_pr` alone is not the gate: the PR that adds the
+  entry dispatches it once.
 
 ## Credentials
 

.github/actions/ext-ci/orchestrator.py

@@ -19,6 +19,7 @@
                   remote runs
 """
 
+import base64
 import json
 import os
 import re
@@ -318,7 +319,8 @@ def relevant_change(files: list[str] | None, manifest: dict) -> bool:
 def select_repos(manifest: dict, target: dict) -> list[dict]:
     """Return the manifest entries to run: all of them, narrowed to the `repos` input ids on a manual run
     (unknown id raises RuntimeError) and to entries with `on_pr` true (the default) when a PR is tested,
-    manual runs included."""
+    manual runs included. Entries the PR itself adds (`new`, see head_manifest) ignore `on_pr`: the PR
+    introducing an entry is the one run it must get."""
     repos = manifest["repos"]
     wanted = (env.get("INPUT_REPOS") or "all").strip()
     if EVENT_NAME == "workflow_dispatch" and wanted and wanted != "all":
@@ -328,10 +330,46 @@ def select_repos(manifest: dict, target: dict) -> list[dict]:
             raise RuntimeError("unknown manifest ids: " + ", ".join(unknown))
         repos = [entry for entry in repos if entry["id"] in ids]
     if target["pr"]:
-        repos = [entry for entry in repos if entry.get("on_pr", True)]
+        repos = [
+            entry for entry in repos if entry.get("on_pr", True) or entry.get("new")
+        ]
     return repos
 
 
+def head_manifest(default: dict, target: dict, files: list[str] | None) -> dict:
+    """The manifest to run from: the one at the F´ revision under test (PR head, merge-queue commit or
+    manual ref), so a PR that adds or edits an entry exercises it on its own checks. It is read as
+    data through the contents API of this repo (PR head commits are reachable here, fork or not); the
+    workflow and this script stay default-branch. When the PR edits manifest.json (`files`), entries
+    whose id the default branch does not have are marked `new`; a stale branch that merely predates a
+    removal gets no such mark. Raises when the file is missing, malformed or has duplicate ids.
+    """
+    try:
+        blob = own.get(
+            f"/repos/{REPO}/contents/{MANIFEST_PATH}?ref={target['head_sha']}"
+        )
+    except ApiError as e:
+        if e.status == 404:
+            raise RuntimeError(
+                f"no {MANIFEST_PATH} at {fprime_rev(target)} (predates external CI?)"
+            ) from e
+        raise
+    head = json.loads(base64.b64decode(blob["content"]))
+    ids = [entry["id"] for entry in head["repos"]]
+    if len(ids) != len(set(ids)):
+        raise RuntimeError(f"duplicate ids in {MANIFEST_PATH} at {fprime_rev(target)}")
+    known = {entry["id"] for entry in default["repos"]}
+    edited = files is None or MANIFEST_PATH in files
+    for entry in head["repos"]:
+        entry["new"] = edited and entry["id"] not in known
+    new = [entry["id"] for entry in head["repos"] if entry["new"]]
+    log(
+        f"manifest.json from {fprime_rev(target)}: {ids}"
+        + (f", new: {new}" if new else "")
+    )
+    return head
+
+
 def resolve_ext_ref(entry: dict, target: dict) -> str:
     """Return the external repo branch to dispatch: its `pr-<N>` branch if it exists, else a branch
     named like the F´ base branch (`devel`, `release/x`), else the entry's `default_ref` (`main`).
@@ -450,7 +488,15 @@ def dispatch_one(manifest: dict, entry: dict, target: dict, state: dict) -> None
     except (
         Exception
     ) as e:  # noqa: BLE001 - one repo's failure must not stop the fan-out
-        s["error"], s["remote_conclusion"] = f"dispatch failed: {e}", "dispatch_failed"
+        hint = (
+            " (no such workflow, or the repository is not in the External CI App installation)"
+            if isinstance(e, ApiError) and e.status in (403, 404)
+            else ""
+        )
+        s["error"], s["remote_conclusion"] = (
+            f"dispatch failed: {e}{hint}",
+            "dispatch_failed",
+        )
         log(f"::error::[{entry['id']}] {s['error']}")
     save_state(state)
 
@@ -463,6 +509,24 @@ def cancel_run(s: dict) -> None:
         log(f"::warning::[{s['id']}] cancel: {e}")
 
 
+def report_waiting(s: dict) -> None:
+    """Retitle one repo's Check Run while its remote run waits for an environment approval."""
+    log(f"[{s['id']}] waiting for approval in {s['repository']}: {s['remote_run_url']}")
+    try:
+        own.patch(
+            f"/repos/{REPO}/check-runs/{s['check_run_id']}",
+            {
+                "output": {
+                    "title": f"Waiting for approval in {s['repository']}",
+                    "summary": f"The remote run is held by a protected environment and needs a reviewer "
+                    f"of {code(s['repository'])}: {s['remote_run_url']}\n\nOrchestrator: {ORCHESTRATOR_URL}",
+                },
+            },
+        )
+    except ApiError as e:  # cosmetic
+        log(f"::warning::[{s['id']}] check update: {e}")
+
+
 def poll_all(state: dict) -> None:
     """Poll every dispatched run every POLL_S seconds until each has a `remote_conclusion`.
 
@@ -484,8 +548,20 @@ def active() -> list[dict]:
                 run = ext.get(
                     f"/repos/{s['repository']}/actions/runs/{s['remote_run_id']}"
                 )
-                if run.get("run_started_at") and not s["remote_started_at"]:
-                    s["remote_started_at"] = run["run_started_at"]
+                if run["status"] == "waiting":
+                    # Held for a protected-environment approval in the external repo (possibly after
+                    # earlier jobs ran): both clocks restart when it is released; the orchestrator
+                    # job's own timeout bounds the wait (finalize cancels it).
+                    s["remote_started_at"] = None
+                    s["dispatched_at"] = now().strftime("%Y-%m-%dT%H:%M:%SZ")
+                    if not s.get("waiting_reported"):
+                        s["waiting_reported"] = True
+                        report_waiting(s)
+                    continue
+                if run["status"] == "in_progress" and not s["remote_started_at"]:
+                    # Observed start, not `run_started_at`: GitHub stamps that at creation, approval
+                    # wait included.
+                    s["remote_started_at"] = now().strftime("%Y-%m-%dT%H:%M:%SZ")
                 timeout = timedelta(minutes=s["timeout_minutes"])
                 if run["status"] == "completed":
                     s["remote_conclusion"] = run["conclusion"]
@@ -546,13 +622,12 @@ def failure_details(s: dict) -> str:
 
 
 def check_conclusion(s: dict) -> str:
-    """Map the remote result to the Check Run conclusion: success, neutral (any failure on a
-    `failure_policy: notify` repo), cancelled, timed_out or failure."""
+    """Map the remote result to the Check Run conclusion: success, cancelled, timed_out or failure.
+    `failure_policy` does not soften it (GitHub folds neutral checks away; a non-blocking failure
+    must still show red): it only decides whether this job fails, see run()."""
     rc = s["remote_conclusion"]
     if rc == "success":
         return "success"
-    if s["failure_policy"] == "notify":
-        return "neutral"  # visible, but does not block the PR
     if rc == "cancelled":
         return "cancelled"
     if rc in ("timed_out", "queue_timeout"):
@@ -579,7 +654,7 @@ def finalize_one(target: dict, s: dict) -> None:
     if s["failure_policy"] == "notify" and s["remote_conclusion"] != "success":
         lines += [
             "",
-            f"This repo is {code('failure_policy: notify')}: the failure does not block the PR.",
+            f"This repo is {code('failure_policy: notify')}: informational, the failure does not block merging.",
         ]
     lines += ["", f"Orchestrator run: {ORCHESTRATOR_URL}"]
     output = {
@@ -676,11 +751,12 @@ def write_summary(target: dict, state: dict) -> None:
 
 # ---------------------------------------------------------------- modes
 def run() -> None:
-    """The `run` mode: resolve target, load manifest, fork preflight, select repos, dispatch, poll,
-    close checks, write summary. Exits 1 when a `failure_policy: block` repo did not succeed or the
-    orchestrator itself failed on any repo."""
+    """The `run` mode: resolve target, fork preflight, load the target's manifest, select repos,
+    dispatch, poll, close checks, write summary. Exits 1 when a `failure_policy: block` repo did not
+    succeed or the orchestrator itself failed on any repo."""
     target = resolve_target()
     log(f"target: {json.dumps(target)}")
+    # The default branch's copy: trusted, used for the preflight below and as the `new` baseline.
     with open(MANIFEST_PATH) as f:
         manifest = json.load(f)
     # Fork preflight: a fork of F´ that copies this workflow must not dispatch into the ext repos.
@@ -691,6 +767,8 @@ def run() -> None:
             "Use workflow_dispatch to run it here deliberately."
         )
         return
+    files = changed_files(target)
+    manifest = head_manifest(manifest, target, files)
     repos = select_repos(manifest, target)
     if ATTEMPT > 1:  # re-run: only the repos whose check is not green yet
         repos = [e for e in repos if not check_succeeded(manifest, e, target)]
@@ -699,9 +777,7 @@ def run() -> None:
         )
     state = {"target": target, "attempt": ATTEMPT, "repos": {}}
     save_state(state)
-    if EVENT_NAME != "workflow_dispatch" and not relevant_change(
-        changed_files(target), manifest
-    ):
+    if EVENT_NAME != "workflow_dispatch" and not relevant_change(files, manifest):
         log("no relevant files changed; marking all repos as skipped")
         skip_all(manifest, target, repos, state)
         write_summary(target, state)
@@ -760,7 +836,7 @@ def finalize() -> None:
         except ApiError:
             pass
         if cancelled:
-            conclusion = "neutral" if s["failure_policy"] == "notify" else "cancelled"
+            conclusion = "cancelled"
             title, why = (
                 "Cancelled by orchestrator",
                 "The orchestrator run was cancelled",
@@ -774,7 +850,7 @@ def finalize() -> None:
             )
             why = "Closed by the finalize step after the check update failed"
         else:
-            conclusion = "neutral" if s["failure_policy"] == "notify" else "failure"
+            conclusion = "failure"
             title = "Orchestrator error"
             why = f"The orchestrator run ended before this repo was reported ({code(s.get('error') or 'orchestrator error')})"
         try: