Repository navigation
ci: restore hosted branch checks - #29
Conversation
Signed-off-by: raghusubramanyam <raghusubramanyam@gmail.com>
Signed-off-by: raghusubramanyam <raghusubramanyam@gmail.com>
agents-lightforge
left a comment
There was a problem hiding this comment.
[forge] Review @7d44f2edd2fb25eb345d33c14da92b9e5d8755ac. Label withheld.
Blocking — this PR's own Branch Checks run 35573293974 (pull-request/29, @7d44f2e) concluded failure: 7/7 real jobs died at Initialize containers, none reached checkout. The runner label was gate 1; pulling ghcr.io/nvidia/openshell/ci:latest with the fork's GITHUB_TOKEN (.github/workflows/branch-checks.yml:43-46) is gate 2, and self-hosted runners were masking it via local image cache. Net effect on #9/#11 is queued -> failure, not unblocked.
Medium — "job names unchanged" is false: :81 and :141 are Rust (${{ matrix.runner }}) / Python (${{ matrix.runner }}), so 4 check-run names move (Rust (linux-amd64-cpu8) -> Rust (ubuntu-24.04)). required-ci-gates.yml:...evaluate_workflow "OpenShell / Branch Checks" passes no required_job_name, so the synthesized context does not key on them; whether branch protection on lightforge-main requires the raw names is unverified — branch-protection is 403 for my token.
Medium — helm-lint.yml:80 still runs-on: linux-amd64-cpu8. Its green on #9/#11 ("Helm Lint passed") is hollow: the helm-lint job skips when no helm path changed and Detect Helm changes alone satisfies the real_success >= 1 counter. Any PR touching deploy/helm/** queues forever.
Minor — linux-amd64-cpu8 (8 vCPU) -> hosted 4 vCPU / 14 GB for the Rust job (build + test + telemetry-off). Unexercised; the run died before compile.
Branch Checks has never completed in this fork (3 runs total: #9 queued, #11 queued, #29 failure). One green Branch Checks run at a mirror head is the condition that lifts the label.
Signed-off-by: raghusubramanyam <raghusubramanyam@gmail.com>
agents-lightforge
left a comment
There was a problem hiding this comment.
[forge] Review @0485d397fa1226787b4914c74b6b59c1911a5866 (supersedes review at 7d44f2e). Label withheld pending a completed Branch Checks run at this head.
Prior Blocking discharged: the 7/7 Initialize containers failure at 7d44f2e is gone. At 8e44be9 run 35573542596 reached 5/7 green (mise Lockfile, License Headers, Markdown, Python x2) before concurrency cancelled it on the re-push; both Rust legs were still in_progress, so Rust is unproven at any head. Run 35573702453 at this head is in progress.
build-cli gate verified rather than accepted: no job declares needs: build-cli and no result-gate job verifies it, so skipping it cannot change any status. Condition is byte-identical to build-supervisor:74; "every other E2E build" is loose — build-gateway:62 uses run_core_e2e || run_kubernetes_ha_e2e.
Medium — the CI pin has no re-pin path. :00797d6468b... is OpenShell #11's head, lightforge-main...00797d64 = ahead 10 / behind 0, so it is not on the base branch. Its only producer is publish-lightforge-connect-closure.yml, trigger workflow_dispatch: alone; ci-image.yml still publishes ghcr.io/nvidia/openshell/ci, fires on push: branches: [main], and runs on linux-amd64-cpu8/linux-arm64-cpu8 — dead in this fork. After #29 lands, a change to Dockerfile.ci/mise.toml/mise.lock/tasks/ has no automated route to a new image, and the stale pin cannot fail loudly.
Medium — pinned by tag, not digest. Re-dispatching the closure at the same SHA republishes :00797d64... and every later branch check silently compiles under a different toolchain. #11 enforces digest pins for the supervisor image via validate_supervisor_image_mount; the image that builds it is now held to the weaker form. Closure run 35570206880 at 00797d6 is still in_progress — its Build source-bound CI image (amd64), (arm64) and Publish source-bound CI manifest jobs did conclude success, which is what carries the multi-arch claim; the run itself is not a receipt.
Medium — helm-lint.yml:80 is still runs-on: linux-amd64-cpu8. Today's OpenShell / Helm Lint | success | Helm Lint passed is hollow: the lint job skips when no helm path changed and Detect Helm changes alone satisfies real_success >= 1 in required-ci-gates.yml. Any PR touching deploy/helm/ queues forever. Same shape for E2E — docker-build.yml:29 defaults to linux-amd64-cpu8, so build-gateway and build-supervisor queue forever on a labeled PR; the two green E2E contexts read not applied, not passed.
Minor — "status names unchanged" holds for the four synthesized contexts but not for the raw check runs: branch-checks.yml:81 and :141 interpolate matrix.runner, so Rust (linux-amd64-cpu8) becomes Rust (ubuntu-24.04) and likewise for Python. evaluate_workflow "OpenShell / Branch Checks" passes no required_job_name, so the gate does not key on them; whether branch protection requires the raw names is unverified, branch-protection is 403 for my token.
Signed-off-by: raghusubramanyam <raghusubramanyam@gmail.com>
|
[hugo] /ok |
agents-lightforge
left a comment
There was a problem hiding this comment.
[forge] Review @50b024729ac5d278452622ae702316d53e9fb54f. Label withheld pending run 35574327505.
Run 35573702453 @0485d397 concluded failure. The image fix is proven: Initialize containers green on all 5 container jobs, 6/8 jobs pass. Both Rust legs died at Lint:
error: redundant closure
--> crates/openshell-policy/src/merge.rs:762:14
= note: `-D clippy::redundant-closure` implied by `-D warnings`
Not introduced by this PR, and not a hosted-runner artifact. That line is byte-identical at lightforge-main, at #9 @7ca725b8 and at #11 @00797d64 — read at each ref, not inferred. It came in with 9ff748eae "feat(policy): scope binary policy by UID and document org isolation" (2026-07-05), fork-authored, and has never been linted because Branch Checks has never completed in this fork. lightforge-main is clippy-red; #29 is the instrument that revealed it.
The fix at this head is correct and semantics-preserving: .iter() yields &NetworkBinary and binary_scope_key already took that, so map(binary_scope_key) is the same call.
Minor — the PR is no longer CI-only; it now carries a crates/ change, so the diff spans two review surfaces. Defensible as the smallest unblock, but say so in the merge note rather than leaving the title ci:.
Minor — this failure is the first concrete cost of the tag pin. -D warnings plus a clippy supplied by ghcr.io/lightforgedev/openshell/ci:00797d64 means a closure re-dispatch at the same SHA with a newer toolchain can turn lightforge-main red with zero source change, and nothing records which clippy the green was obtained under. Reinforces the digest-pin Medium in review 5264091766.
Cited SHA 50b02472fa8b6a99bc6ee8353d1136dd4b85821f returns 404 No commit found; the head is 50b024729ac5d278452622ae702316d53e9fb54f. They share the first 8 characters, so a short-SHA check would have passed. Second instance on this train.
Test and Verify telemetry can be compiled out have been skipped behind a failing Lint at every head so far — hosted runners are still unproven for compile-and-test, and the 4-vCPU concern in review 5264091766 stays unmeasured.
Label lifts on run 35574327505 concluding success at this exact head.
agents-lightforge
left a comment
There was a problem hiding this comment.
[forge] Run 35574327505 at @50b024729ac5d278452622ae702316d53e9fb54f concluded failure. Label still withheld.
6/8 green, both Rust legs at Lint again, identical on amd64 and arm64 — arch is not the discriminator. Four more clippy errors, none of them in this PR's diff (branch-checks.yml, branch-e2e.yml, merge.rs are the only three files):
error: unnested or-patterns
--> crates/openshell-cli/src/run.rs:2797:13
error: struct update has no effect, all the fields in the struct have already been specified
--> crates/openshell-cli/src/run.rs:3317:19
error: this could be rewritten as `let...else`
--> crates/openshell-supervisor-process/src/sandbox/linux/landlock.rs:154:5
error: redundant closure
--> crates/openshell-supervisor-process/src/sandbox/linux/mod.rs:49:19
Blocking on method, not on the code. cargo clippy -p openshell-policy proved one crate; the gate runs the workspace, so a per-crate pass cannot bound what the gate will say. Worse, -D warnings aborted openshell-cli and openshell-supervisor-process, so every crate downstream of those two was never checked — the remaining count is unknown and fixing these four will not necessarily end it. Two heads have now each revealed one round's worth. Enumerate the whole set in one pass under the pinned image before the next push:
docker run --rm -v "$PWD":/w -w /w ghcr.io/lightforgedev/openshell/ci:00797d6468b25b939185af7f53d1952145368f4f mise run rust:lint
That also removes the macOS z3-sys host block you cited, which is why the local run kept disagreeing with CI.
Recommend the lint debt land as its own PR against lightforge-main and #29 revert to CI-only. lightforge-main has been clippy-red since 9ff748eae (2026-07-05) across at least three crates; that is a base-branch defect with its own review surface, and folding it into a ci: PR one error per round trip keeps re-opening this one.
mise run test:rust and rust:verify:telemetry-off remain skipped at every head. Nothing on the hosted runners has compiled the workspace or run a test yet.
Signed-off-by: raghusubramanyam <raghusubramanyam@gmail.com>
|
[hugo] /ok |
agents-lightforge
left a comment
There was a problem hiding this comment.
[forge] Review @6b67bd3b61617a0792c1be42de1ab8baa79024c3. Label withheld pending run 35575083316.
All four rewrites checked for semantic equivalence, not accepted as "Clippy-prescribed":
run.rs:2794— or-pattern nesting only; same three payload arms,| None => {}preserved. Equivalent.landlock.rs:154— the one that needed checking, becauselet...elserequires a diverging else and the original_arm was an expression assigned tolandlock_abi. It diverged on both paths already (return Ok(None)forBestEffort,return Err(...)forHardRequirement), so the rewrite is exact.availabilityis still read inside the else for both messages, andlandlock_abistill reaches its only consumer at:269. No change to the unavailable-Landlock decision or to either OCSF emission.mod.rs:49—as_ref()yields&PreparedRulesetandevidence_envtakes&self, so the path form is the same call.run.rs:3317—ExecSandboxRequestis prost-generated, and..Default::default()is the idiom that keeps proto growth non-breaking at construction sites. Removing it is correct today (every field is named) but converts a future proto field addition from a silent default into a compile error here. Minor, and arguably the better failure mode — flagging it so the choice is deliberate rather than clippy-driven.
Method, third round. You fixed exactly the errors the last run printed, again. -D warnings aborted openshell-cli and openshell-supervisor-process at that run, so every crate downstream of those two was never reached; the set is still not enumerated. cargo fmt --check and git diff --check do not compile anything, so this head carries no local evidence about lint at all. The macOS z3-sys block is not a reason to skip it — the gate runs in a pinned Linux image you can execute directly:
docker run --rm -v "$PWD":/w -w /w ghcr.io/lightforgedev/openshell/ci:00797d6468b25b939185af7f53d1952145368f4f mise run rust:lint
If this run is red again, run that before the next push rather than pushing a fourth time.
test:rust and rust:verify:telemetry-off are still skipped at every head. Diff is now 5 files across 3 crates plus 2 workflows, under a ci: title.
agents-lightforge
left a comment
There was a problem hiding this comment.
[forge] Addendum to review 5264217567, same head @6b67bd3b61617a0792c1be42de1ab8baa79024c3.
The local "full workspace Clippy clean" has a named hole. crates/openshell-supervisor-process/src/sandbox/mod.rs:9-10:
#[cfg(target_os = "linux")]
pub mod linux;On a macOS host, sandbox/linux/landlock.rs and sandbox/linux/mod.rs are not compiled at all — and two of this PR's five repairs live in that tree, including the let...else rewrite. So cargo clippy --workspace --all-targets -- -D warnings passing locally is true for everything not gated on target_os = "linux" and says nothing about the two files carrying the riskiest edit.
It is still a real widening over the aborted CI run for the rest of the workspace, which is what was asked for. Scope the claim in the merge note: the hosted Linux matrix remains the only instrument that has ever compiled sandbox/linux/** in this fork.
agents-lightforge
left a comment
There was a problem hiding this comment.
[forge] Run 35575083316 at @6b67bd3b61617a0792c1be42de1ab8baa79024c3 concluded success. 8/8 jobs. forge-reviewed APPLIED at this exact head.
First completed Branch Checks run in this fork's history — the prior three were #9 queued, #11 queued, #29 failure. Combined status on the head is success, OpenShell / Branch Checks | Branch Checks passed.
What this run proves that no earlier head did: Lint, Test and Verify telemetry can be compiled out all green on both ubuntu-24.04 and ubuntu-24.04-arm. The workspace has now compiled and tested on hosted runners, which closes the 4-vCPU concern I raised at 0485d39 — it was never measured before because Test was skipped behind a failing Lint at every head. It also covers sandbox/linux/**, the tree your macOS clippy run could not reach.
Three gates were in series and each masked the next: runner labels with no runners, an unpullable ghcr.io/nvidia/openshell/ci:latest, and lightforge-main clippy-red since 9ff748eae (2026-07-05) across three crates. All three are discharged at this head.
Carried forward as follow-up, not blockers on this PR, per agreement:
publish-lightforge-connect-closure.ymlisworkflow_dispatch:-only andci-image.ymltargets the NVIDIA namespace onmainwith self-hosted runners, so the CI pin has no automated re-pin path.- The pin is a tag, not a digest. A closure re-dispatch at the same SHA with a newer toolchain can turn
lightforge-mainred with zero source change — this PR's own clippy history is the demonstration. helm-lint.yml:80anddocker-build.yml:29still carrylinux-amd64-cpu8, so adeploy/helm/**PR or a labeled E2E PR still queues forever. Today'sHelm Lint passedis a skip, not a run.- Minor: removing
..Default::default()from the prost-generatedExecSandboxRequestturns future proto field additions into a compile error atrun.rs:3317.
Merge note should state the expanded scope: 5 files across 3 crates plus 2 workflows under a ci: title.
After this lands, expect #9 and #11 to move from queued to running rather than to green in one step — neither has ever had a completed Branch Checks run either.
Summary
Restore the Lightforge fork Branch Checks on GitHub-hosted x64 and arm64 runners. NVIDIA-only labels and the NVIDIA-private CI image left required checks queued or unable to initialize, blocking reviewed PRs #9 and #11.
Related Issue
N/A: issues are disabled in this fork.
Changes
ubuntu-24.04andubuntu-24.04-arm.ghcr.io/lightforgedev/openshell/ci:00797d6468b25b939185af7f53d1952145368f4f.Testing
git diff --checkandcargo fmt --all -- --checkpass.mise run rust:lintpasses locally with Rust/Clippy 1.95 after supplying a SHA-256-verified Z3 5.1.0 bottle; macOS excludessandbox/linux/**.Checklist