Repository navigation
Fix packed golden recovery and live debug socket routing - #103
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 471cd9d33a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| &job_id, | ||
| ExecutionStatus::Cancelled, | ||
| ) | ||
| .await; |
There was a problem hiding this comment.
When a GitHub-backed run cancels multiple jobs, this loop awaits every external check-run PATCH sequentially before emitting RunStatus or returning the cancellation response. Since shared_http::CLIENT permits 30 seconds per request, a slow or unreachable GitHub API multiplies the delay by the number of cancelled jobs, leaving existing event-stream clients apparently stuck even though the in-memory run is already terminal. Broadcast the cancellation first and perform these best-effort updates concurrently or in the background.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 6 files (changes from recent commits).
Confidence score: 4/5
- In
scripts/preloop-e2e-bench.sh, embedding the runner token directly into Python source can break registration when the token contains quote/escape characters, which can cause flaky or failed E2E setup in real environments — pass the token viaos.environand export it before invocation. - In
crates/preloop-runner-server/src/lib_tests.rs, test env state is restored only after manual cleanup, so a panic before that point can leak mock URL/token into later tests and trigger misleading failures or accidental credential reuse — preserve and restore env values with panic-safe cleanup/RAII. - In
crates/preloop-runner-server/src/runs.rs, cancelled-job completion updates are sent in a serial await loop, so large cancellation bursts can stall run finalization and increase API-latency exposure — dispatch thesereport_check_run_completedcalls concurrently (with a reasonable limit). - In
docs/conformance.md, a stray non-breaking space introduces an unexpected non-ASCII character in an otherwise clean line, which can cause avoidable lint/diff noise — remove the U+00A0 character.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/preloop-runner-server/src/lib_tests.rs">
<violation number="1" location="crates/preloop-runner-server/src/lib_tests.rs:5066">
P3: concern: A panic/failing assertion before manual cleanup leaves this mock URL and token installed after releasing `GITHUB_ENV_LOCK`, so later tests can call a dead mock or use the test credential; preserve and restore both variables with a Drop guard.</violation>
</file>
<file name="docs/conformance.md">
<violation number="1" location="docs/conformance.md:143">
P3: nit: the only change on this line adds a stray trailing non-breaking space (U+00A0) after the closing backtick. This looks unintentional and introduces a non-ASCII character into an otherwise unchanged documentation sentence; drop it so the line ends with the period.</violation>
</file>
<file name="scripts/preloop-e2e-bench.sh">
<violation number="1" location="scripts/preloop-e2e-bench.sh:104">
P2: concern: Configured system tokens containing Python-string characters make runner registration fail because this shell expansion is embedded as Python source. Pass the token through `os.environ` (and export it) rather than interpolating it into the script.</violation>
</file>
<file name="crates/preloop-runner-server/src/runs.rs">
<violation number="1" location="crates/preloop-runner-server/src/runs.rs:2117">
P3: The cancelled-jobs check-run completion reports are awaited one at a time in a serial loop. Each `report_check_run_completed` issues an HTTP PATCH to the GitHub API, so for a run that cancels many jobs this blocks the `/cancel` response on a long chain of sequential network round trips. Since these calls are independent (the inner lock is already dropped and none mutate shared queue state), they can be fanned out, e.g. with `futures::future::join_all`, so the cancel endpoint returns promptly and check updates go out concurrently.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| headers={'Content-Type':'application/json','Authorization':'RemoteAuth test'}, | ||
| headers={ | ||
| 'Content-Type':'application/json', | ||
| 'Authorization':'RemoteAuth ${REGISTRATION_TOKEN}', |
There was a problem hiding this comment.
P2: concern: Configured system tokens containing Python-string characters make runner registration fail because this shell expansion is embedded as Python source. Pass the token through os.environ (and export it) rather than interpolating it into the script.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/preloop-e2e-bench.sh, line 104:
<comment>concern: Configured system tokens containing Python-string characters make runner registration fail because this shell expansion is embedded as Python source. Pass the token through `os.environ` (and export it) rather than interpolating it into the script.</comment>
<file context>
@@ -98,7 +99,10 @@ import urllib.request, json
- headers={'Content-Type':'application/json','Authorization':'RemoteAuth test'},
+ headers={
+ 'Content-Type':'application/json',
+ 'Authorization':'RemoteAuth ${REGISTRATION_TOKEN}',
+ },
method='POST'
</file context>
| }); | ||
|
|
||
| let _env = crate::state::GITHUB_ENV_LOCK.lock().await; | ||
| std::env::set_var("PRELOOP_GITHUB_API_URL", format!("http://127.0.0.1:{port}")); |
There was a problem hiding this comment.
P3: concern: A panic/failing assertion before manual cleanup leaves this mock URL and token installed after releasing GITHUB_ENV_LOCK, so later tests can call a dead mock or use the test credential; preserve and restore both variables with a Drop guard.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/preloop-runner-server/src/lib_tests.rs, line 5066:
<comment>concern: A panic/failing assertion before manual cleanup leaves this mock URL and token installed after releasing `GITHUB_ENV_LOCK`, so later tests can call a dead mock or use the test credential; preserve and restore both variables with a Drop guard.</comment>
<file context>
@@ -5040,6 +5040,131 @@ jobs:
+ });
+
+ let _env = crate::state::GITHUB_ENV_LOCK.lock().await;
+ std::env::set_var("PRELOOP_GITHUB_API_URL", format!("http://127.0.0.1:{port}"));
+ std::env::set_var("PRELOOP_GITHUB_TOKEN", "cancel-test-token");
+
</file context>
| specification of the server's scheduling/gate logic from the Rust source, | ||
| repaired it against TLC's strict typing during validation, and hunted bugs | ||
| with real SANY + TLC runs (`experiments/specula-20260804/`). | ||
| with real SANY + TLC runs (`experiments/specula-20260804/`). |
There was a problem hiding this comment.
P3: nit: the only change on this line adds a stray trailing non-breaking space (U+00A0) after the closing backtick. This looks unintentional and introduces a non-ASCII character into an otherwise unchanged documentation sentence; drop it so the line ends with the period.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/conformance.md, line 143:
<comment>nit: the only change on this line adds a stray trailing non-breaking space (U+00A0) after the closing backtick. This looks unintentional and introduces a non-ASCII character into an otherwise unchanged documentation sentence; drop it so the line ends with the period.</comment>
<file context>
@@ -140,7 +140,7 @@ the [Specula](https://github.com/SpeculaIO/Specula) pipeline (code analysis
specification of the server's scheduling/gate logic from the Rust source,
repaired it against TLC's strict typing during validation, and hunted bugs
-with real SANY + TLC runs (`experiments/specula-20260804/`).
+with real SANY + TLC runs (`experiments/specula-20260804/`).
**Six findings, all fixed and reconciled into the current tree** (2026-08-06):
</file context>
| with real SANY + TLC runs (`experiments/specula-20260804/`). | |
| with real SANY + TLC runs (`experiments/specula-20260804/`). |
| if cancellation_count > 0 { | ||
| shared.state.message_notify.notify_waiters(); | ||
| } | ||
| for job_id in cancelled_jobs { |
There was a problem hiding this comment.
P3: The cancelled-jobs check-run completion reports are awaited one at a time in a serial loop. Each report_check_run_completed issues an HTTP PATCH to the GitHub API, so for a run that cancels many jobs this blocks the /cancel response on a long chain of sequential network round trips. Since these calls are independent (the inner lock is already dropped and none mutate shared queue state), they can be fanned out, e.g. with futures::future::join_all, so the cancel endpoint returns promptly and check updates go out concurrently.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/preloop-runner-server/src/runs.rs, line 2117:
<comment>The cancelled-jobs check-run completion reports are awaited one at a time in a serial loop. Each `report_check_run_completed` issues an HTTP PATCH to the GitHub API, so for a run that cancels many jobs this blocks the `/cancel` response on a long chain of sequential network round trips. Since these calls are independent (the inner lock is already dropped and none mutate shared queue state), they can be fanned out, e.g. with `futures::future::join_all`, so the cancel endpoint returns promptly and check updates go out concurrently.</comment>
<file context>
@@ -2087,15 +2088,41 @@ pub(crate) async fn cancel_run(
if cancellation_count > 0 {
shared.state.message_notify.notify_waiters();
}
+ for job_id in cancelled_jobs {
+ crate::github::report_check_run_completed(
+ &shared,
</file context>
There was a problem hiding this comment.
5 issues found across 3 files (changes from recent commits).
Confidence score: 2/5
- In
crates/preloop-orchestrator/src/lib.rs, the alternatives registration leaves unversionedclang/clang++/format/tidy effectively pointing to Clang 16 instead of the hosted default 18, which can change toolchain behavior across jobs and cause unexpected build/lint regressions — explicitly set the intended default alternative after registration. - In
crates/preloop-orchestrator/src/lib.rs, the new happy path no longer verifies that clang/gcc aliases resolve to the pinned versions, so silent version drift can pass setup and surface later as hard-to-diagnose compile or test failures — restore explicit post-install version checks before continuing. - In
crates/preloop-orchestrator/src/lib.rs, the fallback path currently treats anyapt-getnon-zero exit as pin drift, so transient package-manager failures may trigger unnecessary baseline installs and mask the real outage cause — gate fallback on concrete pin-mismatch signals and fail fast on transient infra errors. - In
crates/preloop-runner-server/src/github.rs, skip-workflow filtering is applied for webhook submissions but not for scheduler reconciliation, so github-owned workflows may still be scheduled through another entry path; the related env-var test also leaves global state vulnerable on early assertion failure — apply skip config consistently in both paths and use scoped env guards/RAII cleanup in tests.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/preloop-runner-server/src/github.rs">
<violation number="1" location="crates/preloop-runner-server/src/github.rs:1021">
P2: The new skip config is only applied to webhook-triggered submissions, but the same github-owned workflows are still handed to the scheduler unchanged. Earlier in this loop, `scheduler.reconcile_all(&workflows, ...)` runs over the complete inventory (including `release.yml`/artifact workflows) before the per-file skip, and `reconcile_all`/`reconcile` register cron schedules for every workflow that defines `on: schedule`. So a deployment that sets `PRELOOP_GITHUB_SKIP_WORKFLOWS` to keep release/artifact workflows on GitHub will still see those workflows fire locally on their schedule triggers, which partially defeats the stated intent of the config. Consider filtering the github-owned workflows out of the inventory before the scheduler reconciliation (and before `fetch_workflows` submission) so the config consistently applies to both the webhook dispatcher and the scheduler.</violation>
<violation number="2" location="crates/preloop-runner-server/src/github.rs:1656">
P3: The test mutates the process-global `PRELOOP_GITHUB_SKIP_WORKFLOWS` env var and only restores it with `remove_var` at the very end of the test body. If any earlier assertion in this test fails (e.g. `assert_eq!(... StatusCode::OK)` or `assert!(inner.runs.is_empty())`), the env var is left set for subsequent tests in the same process, silently making every other webhook test skip any `release.yml` workflow. Prefer a guard that removes the var on drop (even on panic), or restructure so the env mutation is scoped.</violation>
</file>
<file name="crates/preloop-orchestrator/src/lib.rs">
<violation number="1" location="crates/preloop-orchestrator/src/lib.rs:649">
P2: The baseline fallback triggers on any non-zero apt-get exit, not just genuine pin drift. The `if ! apt-get install {base_packages_pinned}` treats a transient failure (dpkg lock contention, flaky mirror, interrupted fetch during start-up) the same as an unavailable exact pin, and then permanently bakes unpinned archive versions into the golden. That silently erodes the exact-version parity with `ubuntu-latest` that these pins and the fidelity tests exist to guarantee. Recommend distinguishing the two cases: only fall back when apt reports the pin itself is unavailable (e.g. grep for `Unable to locate package`/`has no installation candidate` and the pinned name, or `apt-get -s simulate` the pinned list first), and otherwise fail so a transient error is surfaced and retried rather than masked as a version downgrade.</violation>
<violation number="2" location="crates/preloop-orchestrator/src/lib.rs:666">
P2: The happy path lost all version verification. The previous script confirmed clang-16/17/18 resolved to the exact pinned versions and gcc-12/13/14 to their exact versions before continuing; the new `then` branch registers update-alternatives entries without ever checking what the binaries actually are. Fallback degradation when the archive has genuinely drifted is reasonable, but the success path should still assert the pinned versions match `versions.toml` (clang 16.0.6/17.0.6/18.1.3, gcc 12.4.0/13.3.0/14.2.0) so an in-range archive that resolves a different build cannot bake a silently divergent compiler matrix. Consider restoring the version greps/tests inside the `then` branch.</violation>
<violation number="3" location="crates/preloop-orchestrator/src/lib.rs:667">
P1: blocker: Unversioned `clang`, `clang++`, format, and tidy now resolve to Clang 16 instead of hosted default 18 because all alternatives have equal priority. Set the configured default explicitly after registering the matrix.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| test \"$(gcc-14 -dumpfullversion)\" = '{GCC_14_VERSION}') && \ | ||
| if DEBIAN_FRONTEND=noninteractive \ | ||
| apt-get install -y -qq --no-install-recommends {compiler_packages}; then \ | ||
| for version in {CLANG_VERSIONS}; do \ |
There was a problem hiding this comment.
P1: blocker: Unversioned clang, clang++, format, and tidy now resolve to Clang 16 instead of hosted default 18 because all alternatives have equal priority. Set the configured default explicitly after registering the matrix.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/preloop-orchestrator/src/lib.rs, line 667:
<comment>blocker: Unversioned `clang`, `clang++`, format, and tidy now resolve to Clang 16 instead of hosted default 18 because all alternatives have equal priority. Set the configured default explicitly after registering the matrix.</comment>
<file context>
@@ -657,19 +662,33 @@ pub fn base_install_script() -> String {
- test \"$(gcc-14 -dumpfullversion)\" = '{GCC_14_VERSION}') && \
+ if DEBIAN_FRONTEND=noninteractive \
+ apt-get install -y -qq --no-install-recommends {compiler_packages}; then \
+ for version in {CLANG_VERSIONS}; do \
+ if [ -x /usr/bin/clang++-$version ]; then \
+ update-alternatives --install /usr/bin/clang++ clang++ /usr/bin/clang++-$version 100 || true; \
</file context>
| } | ||
|
|
||
| for (filename, content) in workflows { | ||
| if is_github_owned_workflow(&filename, &github_owned_workflows) { |
There was a problem hiding this comment.
P2: The new skip config is only applied to webhook-triggered submissions, but the same github-owned workflows are still handed to the scheduler unchanged. Earlier in this loop, scheduler.reconcile_all(&workflows, ...) runs over the complete inventory (including release.yml/artifact workflows) before the per-file skip, and reconcile_all/reconcile register cron schedules for every workflow that defines on: schedule. So a deployment that sets PRELOOP_GITHUB_SKIP_WORKFLOWS to keep release/artifact workflows on GitHub will still see those workflows fire locally on their schedule triggers, which partially defeats the stated intent of the config. Consider filtering the github-owned workflows out of the inventory before the scheduler reconciliation (and before fetch_workflows submission) so the config consistently applies to both the webhook dispatcher and the scheduler.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/preloop-runner-server/src/github.rs, line 1021:
<comment>The new skip config is only applied to webhook-triggered submissions, but the same github-owned workflows are still handed to the scheduler unchanged. Earlier in this loop, `scheduler.reconcile_all(&workflows, ...)` runs over the complete inventory (including `release.yml`/artifact workflows) before the per-file skip, and `reconcile_all`/`reconcile` register cron schedules for every workflow that defines `on: schedule`. So a deployment that sets `PRELOOP_GITHUB_SKIP_WORKFLOWS` to keep release/artifact workflows on GitHub will still see those workflows fire locally on their schedule triggers, which partially defeats the stated intent of the config. Consider filtering the github-owned workflows out of the inventory before the scheduler reconciliation (and before `fetch_workflows` submission) so the config consistently applies to both the webhook dispatcher and the scheduler.</comment>
<file context>
@@ -990,6 +1018,14 @@ async fn process_github_webhook(
}
for (filename, content) in workflows {
+ if is_github_owned_workflow(&filename, &github_owned_workflows) {
+ info!(
+ workflow = %filename,
</file context>
| test \"$(gcc-13 -dumpfullversion)\" = '{GCC_13_VERSION}' && \ | ||
| test \"$(gcc-14 -dumpfullversion)\" = '{GCC_14_VERSION}') && \ | ||
| if DEBIAN_FRONTEND=noninteractive \ | ||
| apt-get install -y -qq --no-install-recommends {compiler_packages}; then \ |
There was a problem hiding this comment.
P2: The happy path lost all version verification. The previous script confirmed clang-16/17/18 resolved to the exact pinned versions and gcc-12/13/14 to their exact versions before continuing; the new then branch registers update-alternatives entries without ever checking what the binaries actually are. Fallback degradation when the archive has genuinely drifted is reasonable, but the success path should still assert the pinned versions match versions.toml (clang 16.0.6/17.0.6/18.1.3, gcc 12.4.0/13.3.0/14.2.0) so an in-range archive that resolves a different build cannot bake a silently divergent compiler matrix. Consider restoring the version greps/tests inside the then branch.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/preloop-orchestrator/src/lib.rs, line 666:
<comment>The happy path lost all version verification. The previous script confirmed clang-16/17/18 resolved to the exact pinned versions and gcc-12/13/14 to their exact versions before continuing; the new `then` branch registers update-alternatives entries without ever checking what the binaries actually are. Fallback degradation when the archive has genuinely drifted is reasonable, but the success path should still assert the pinned versions match `versions.toml` (clang 16.0.6/17.0.6/18.1.3, gcc 12.4.0/13.3.0/14.2.0) so an in-range archive that resolves a different build cannot bake a silently divergent compiler matrix. Consider restoring the version greps/tests inside the `then` branch.</comment>
<file context>
@@ -657,19 +662,33 @@ pub fn base_install_script() -> String {
- test \"$(gcc-13 -dumpfullversion)\" = '{GCC_13_VERSION}' && \
- test \"$(gcc-14 -dumpfullversion)\" = '{GCC_14_VERSION}') && \
+ if DEBIAN_FRONTEND=noninteractive \
+ apt-get install -y -qq --no-install-recommends {compiler_packages}; then \
+ for version in {CLANG_VERSIONS}; do \
+ if [ -x /usr/bin/clang++-$version ]; then \
</file context>
| DEBIAN_FRONTEND=noninteractive \ | ||
| apt-get install -y -qq --no-install-recommends {base_packages_pinned} \ | ||
| (echo \"### install hosted apt baseline\" >&2 && \ | ||
| if ! DEBIAN_FRONTEND=noninteractive \ |
There was a problem hiding this comment.
P2: The baseline fallback triggers on any non-zero apt-get exit, not just genuine pin drift. The if ! apt-get install {base_packages_pinned} treats a transient failure (dpkg lock contention, flaky mirror, interrupted fetch during start-up) the same as an unavailable exact pin, and then permanently bakes unpinned archive versions into the golden. That silently erodes the exact-version parity with ubuntu-latest that these pins and the fidelity tests exist to guarantee. Recommend distinguishing the two cases: only fall back when apt reports the pin itself is unavailable (e.g. grep for Unable to locate package/has no installation candidate and the pinned name, or apt-get -s simulate the pinned list first), and otherwise fail so a transient error is surfaced and retried rather than masked as a version downgrade.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/preloop-orchestrator/src/lib.rs, line 649:
<comment>The baseline fallback triggers on any non-zero apt-get exit, not just genuine pin drift. The `if ! apt-get install {base_packages_pinned}` treats a transient failure (dpkg lock contention, flaky mirror, interrupted fetch during start-up) the same as an unavailable exact pin, and then permanently bakes unpinned archive versions into the golden. That silently erodes the exact-version parity with `ubuntu-latest` that these pins and the fidelity tests exist to guarantee. Recommend distinguishing the two cases: only fall back when apt reports the pin itself is unavailable (e.g. grep for `Unable to locate package`/`has no installation candidate` and the pinned name, or `apt-get -s simulate` the pinned list first), and otherwise fail so a transient error is surfaced and retried rather than masked as a version downgrade.</comment>
<file context>
@@ -645,8 +645,13 @@ fn node_externals_at(runner_root: &str) -> Vec<Vec<String>> {
- DEBIAN_FRONTEND=noninteractive \
- apt-get install -y -qq --no-install-recommends {base_packages_pinned} \
+ (echo \"### install hosted apt baseline\" >&2 && \
+ if ! DEBIAN_FRONTEND=noninteractive \
+ apt-get install -y -qq --no-install-recommends {base_packages_pinned}; then \
+ echo \"WARNING: exact hosted apt pins are unavailable; falling back to archive versions\" >&2; \
</file context>
| #[tokio::test] | ||
| async fn webhook_skips_github_owned_workflows() { | ||
| let _env = crate::state::GITHUB_ENV_LOCK.lock().await; | ||
| std::env::set_var(GITHUB_OWNED_WORKFLOWS_ENV, "release.yml"); |
There was a problem hiding this comment.
P3: The test mutates the process-global PRELOOP_GITHUB_SKIP_WORKFLOWS env var and only restores it with remove_var at the very end of the test body. If any earlier assertion in this test fails (e.g. assert_eq!(... StatusCode::OK) or assert!(inner.runs.is_empty())), the env var is left set for subsequent tests in the same process, silently making every other webhook test skip any release.yml workflow. Prefer a guard that removes the var on drop (even on panic), or restructure so the env mutation is scoped.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/preloop-runner-server/src/github.rs, line 1656:
<comment>The test mutates the process-global `PRELOOP_GITHUB_SKIP_WORKFLOWS` env var and only restores it with `remove_var` at the very end of the test body. If any earlier assertion in this test fails (e.g. `assert_eq!(... StatusCode::OK)` or `assert!(inner.runs.is_empty())`), the env var is left set for subsequent tests in the same process, silently making every other webhook test skip any `release.yml` workflow. Prefer a guard that removes the var on drop (even on panic), or restructure so the env mutation is scoped.</comment>
<file context>
@@ -1594,6 +1648,36 @@ mod tests {
+ #[tokio::test]
+ async fn webhook_skips_github_owned_workflows() {
+ let _env = crate::state::GITHUB_ENV_LOCK.lock().await;
+ std::env::set_var(GITHUB_OWNED_WORKFLOWS_ENV, "release.yml");
+
+ let temp = tempfile::tempdir().unwrap();
</file context>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 42608c4d67
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| for (filename, content) in workflows { | ||
| if is_github_owned_workflow(&filename, &github_owned_workflows) { |
There was a problem hiding this comment.
With --enable-scheduler and a skipped workflow containing on: schedule, this filter runs only after the complete workflows map has already been passed to Scheduler::reconcile_all above; that method registers every workflow's cron, and Scheduler::scan_workspace also registers them unfiltered at startup. Consequently a GitHub-owned release or artifact workflow can still execute locally on its schedule and potentially publish twice. Apply the ownership filter to scheduler reconciliation and startup scanning as well.
Useful? React with 👍 / 👎.
| for version in {CLANG_VERSIONS}; do \ | ||
| if [ -x /usr/bin/clang++-$version ]; then \ | ||
| update-alternatives --install /usr/bin/clang++ clang++ /usr/bin/clang++-$version 100 || true; \ |
There was a problem hiding this comment.
On a clean golden where the pinned compiler install succeeds, this loop registers Clang 16, 17, and 18 with the same priority of 100. The local update-alternatives --help states that “options with higher numbers have higher priority in automatic mode”; with these equal priorities, installing the paths in this order leaves /usr/bin/clang and the related tools selecting version 16 rather than the configured CLANG_DEFAULT_VERSION of 18. Any unmodified workflow invoking clang, clang++, or clang-format therefore gets an older compiler than the intended hosted-image default; register only version 18 or give it a higher priority.
AGENTS.md reference: AGENTS.md:L59-L59
Useful? React with 👍 / 👎.
| else \ | ||
| echo \"WARNING: hosted compiler matrix is not available in this Ubuntu archive; continuing with the base compiler toolchain\" >&2; \ | ||
| for package in clang clang-format clang-tidy gcc g++ gfortran; do \ | ||
| DEBIAN_FRONTEND=noninteractive \ | ||
| apt-get install -y -qq --no-install-recommends \"$package\" || \ | ||
| echo \"compiler package unavailable: $package\" >&2; \ |
There was a problem hiding this comment.
When even one package in the aggregate versioned compiler install is unavailable, this fallback abandons the entire requested matrix and installs only the archive-default clang, gcc, and related packages. For example, an unavailable clang-18 causes otherwise available commands such as clang-16, clang-17, gcc-12, and gcc-13 to be omitted as well, so workflows that explicitly invoke those hosted-image tools fail even though the archive could provide them. Attempt the requested versioned packages independently before falling back to unversioned defaults.
AGENTS.md reference: AGENTS.md:L59-L59
Useful? React with 👍 / 👎.
Summary
Verification
just test-ciSummary by cubic
Recovers packed-golden runner forks by falling back to direct creation, hardens base installs against apt drift, restricts the control socket to worker-only live debug, completes cancellation bookkeeping, and preserves action refs across restarts. Adds an opt-out for GitHub-owned workflows in webhook processing.
preloop-orchestrator: On default packed-golden fork failure, clean up any clone and create from the pack; environment-specific goldens still error. Base install falls back when exact apt pins or compiler packages are missing.preloop-runner-server: Control socket admits only worker debug routes (token exchange, open, verdict poll, close); controller/agent debug and other APIs stay denied.preloop-runner-server: Run cancellation sets terminal status and completed_at, completes GitHub Checks as “cancelled,” and refreshes scheduler (queue depth + next-job labels).preloop-runner-server: Webhook dispatcher can skip GitHub-owned workflows viaPRELOOP_GITHUB_SKIP_WORKFLOWS(by filename or.github/workflows/...path).preloop-gha-protocol: Preserve actionrefacross wire roundtrips; encode with canonicalrefand accept it when decoding.Written for commit 30fb871. Summary will update on new commits.