Skip to content

fix(ci): verify patched FreeBSD Node runtime before native proof - #28

Open
joshavant wants to merge 1 commit into
mainfrom
fix/freebsd-runtime-admission
Open

joshavant wants to merge 1 commit into
mainfrom
fix/freebsd-runtime-admission

Conversation

@joshavant

@joshavant joshavant commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

FreeBSD native CI installs major-line pkg packages, but its previous guard checked only platform and architecture. An exact patched matrix label therefore did not establish the runtime actually used for native proof.

Check the selected executable, stable Node version, platform, architecture and same-major patched floor before native builds/tests. Preserve newer stable patches within the selected major. Refusal records the actual identity and gives package-upgrade guidance. This is CI admission only; package APIs, engines and application runtime policy are unchanged.

Validation:

  • Frozen original FreeBSD guard admitted below-floor 22.23.3 / 24.22.0 with controlled process facts. This is source-level reproduction, not native FreeBSD execution.
  • A behavior-preserving extraction of the original guard failed the actual spawned-CLI floor regression before the repair; the repaired owner passes all three tests. The principal red failure is an admitted below-floor runtime, not a missing helper.
  • Actual official checksum-verified Node binaries on isolated native Linux ARM64: 22.23.3 / 24.22.0 rejected; 22.23.4 / 24.22.1 accepted. Wrong-major selection rejected; newer same-major patch accepted. No HTTPS was exercised under affected runtimes.
  • Full pnpm check with declared pnpm 12.15.0 and patched Node 26.12.1: lint, typecheck, build, 107 tests passed / 15 platform-native skips, package-manifest checks passed. Skips do not establish native behavior.
  • actionlint 1.7.12 with checked-in .github/actionlint.yaml, shellcheck and git diff --check passed. Initial actionlint omitted the checked-in runner config and reported the existing custom xcode-27 label; the corrected invocation passed without exceptions.
  • Independent P0–P2 review of the exact four-file candidate: scoped-clean, one complete attempt.

Hosted exact-head result: 20 checks succeeded; all four FreeBSD native jobs stopped at the new runtime admission guard before native build/test. The pkg-selected executable /usr/local/bin/node was 22.23.2 for both Node22 x64/ARM64 jobs (required >=22.23.4 in that major), and 24.21.0 for both Node24 jobs (required >=24.22.1). These are intentional refusals of affected runtimes, not passing native qualification or evidence that the failures are unrelated. Native FreeBSD product execution remains blocked until patched same-major pkg runtimes are available or a separately qualified build route is authorized. No guard, native test or required gate was weakened or skipped.

Run: https://github.com/openclaw/proc-safe/actions/runs/40285117746 (failed FreeBSD jobs121446823325,121446823441,121446823431,121446823443). This PR does not establish TLS-mode behavior, release delivery or fleet remediation, and does not publish a release or package.

@clawsweeper

clawsweeper Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. labels Oct 26, 2026
@clawsweeper

clawsweeper Bot commented Oct 26, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge.

What this changes

This PR verifies FreeBSD Node platform, architecture, major version, and minimum patch level before native builds and tests.

Example: The FreeBSD x64 Node 22 job installs Node 22.23.3 against a 22.23.4 matrix floor.

  • Before: The platform and architecture guard accepts it and native proof continues.
  • After: The guard reports the executable and runtime identity, gives package-upgrade guidance, and exits before native builds.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The patch is focused and source-correct; the reported CLI exercise and spawned-process tests provide limited qualification of the FreeBSD workflow.
Proof confidence 🦐 gold shrimp (3/6) The supplied maintainer-authored context exempts ordinary contributor proof: the body reports actual Linux ARM64 Node CLI acceptance and rejection, while FreeBSD execution remains unqualified; blocked check-log access creates no contributor proof obligation.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Maintainers can receive successful FreeBSD native results under a patched matrix label even though pkg supplied an older Node runtime.
Reason: The change makes runtime qualification truthful through a focused guard; the supplied maintainer-authored context settles the intentional refusal of below-floor packages.

Merge readiness

✅ Ready for maintainer review

Keep open: current main does not verify the installed FreeBSD Node patch level. The focused change has no identified correctness defect, but the four failing FreeBSD checks remain unattributed.

Priority: P2
Reviewed head: b5d97a49a3e02b5104301769a2acb11b2eea5f60

Before merge

None.

Findings

None.

Tests

  • Missing end-to-end proof: The supplied Linux CLI account does not establish hosted FreeBSD admission and native continuation; the failing VM jobs' raw logs were inaccessible from this review environment.
Agent review details

How this fits together

The reusable native workflow installs Node from FreeBSD pkg, checks its runtime identity, then builds, tests, and packages the native binding; both CI and release prebuilds consume this workflow.

flowchart TD
  A[CI or release prebuild] --> B[FreeBSD VM]
  B --> C[Major-line pkg installation]
  C --> D[Runtime identity and version guard]
  D --> E[Reject with upgrade guidance]
  D --> F[Native build and tests]
  F --> G[Binding artifact]
Loading

Technical review

Best possible solution:

Keep the focused runtime guard and establish the cause and disposition of the FreeBSD check failures before landing.

Do we have a high-confidence way to reproduce the issue?

Main's guard has no version comparison, so a FreeBSD runtime with the expected architecture passes regardless of its patch level; this is source-established without executing target code.

Is this the best way to solve the issue?

The small CLI guard enforces the existing matrix floor at the VM boundary, accepts newer stable versions within the selected major, and leaves package APIs and engines unchanged.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 52244652b89f.

Provenance checked

  • FreeBSD platform and architecture guard keeps the original intent (5852b3e: The scaffold established real FreeBSD kernel coverage and checked that the VM ran the intended platform and architecture.)
  • Patched FreeBSD matrix versus major-line package installation changes intended behavior with a stated reason (5224465: The merged toolchain hardening retained FreeBSD coverage while explicitly deferring qualification of unavailable patched packages; this PR explains why older runtime results must now be refused.)

Testing

Proof path: unit tests only.

Security

None.

Evidence

What I checked:

  • Verified PR scope: The pinned introduction diff changes only the FreeBSD guard, its documentation, a 15-line CLI script, and focused spawned-process tests; the current main branch remains at the pinned base. (.github/workflows/native.yml:242, b5d97a49a3e0)
  • Existing runtime qualification gap: Main selects patched matrix versions but installs major-line FreeBSD packages and checks only platform and architecture; its toolchain-hardening commit explicitly leaves patched FreeBSD qualification as follow-up work. (.github/workflows/native.yml:239, 52244652b89f)
  • Related merged work: fix(ci): use Node and pnpm releases that reject compromised CA keys #27 records available FreeBSD packages below the selected floors and distinguishes passing native tests from patched-runtime qualification; this PR addresses that admission gap rather than duplicating the merged update. (.github/workflows/native.yml:212, 52244652b89f)
  • Fail-closed behavior is intentional: The complete PR body expressly accepts refusing native proof when pkg supplies an older patch; the supplied review context identifies this as maintainer-authored direction, although live REST author_association reports CONTRIBUTOR. (scripts/check-ci-runtime.mjs:11, b5d97a49a3e0)
  • Current check failures are not attributed: All four FreeBSD jobs passed Linux preparation and failed inside the VM step; the inspected annotation says only 'ssh exited with code 1'. Raw log retrieval redirected to a blocked storage host, so neither intended version refusal nor another cause is established. (.github/workflows/native.yml:233, b5d97a49a3e0)
  • Repository policy applied: The full root AGENTS.md was read; no nested AGENTS.md, maintainer-notes directory, or pr-simplify skill was found. The guard remains CI-only, with package APIs and runtime policy unchanged; maintenance-only CI work needs no changelog entry. No target code, tests, build, or second reviewer ran, and the checkout remains clean. (AGENTS.md:1, b5d97a49a3e0)

Likely related people:

  • Josh Avant: Raw commit 5224465 adds .github/workflows/native.yml:212 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 52244652b89f; files: .github/workflows/native.yml)
  • Peter Steinberger: Raw commit 5852b3e adds .github/workflows/native.yml:147 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 5852b3e263bf; files: .github/workflows/native.yml)

Labels

Label changes:

  • add P2: This closes a concrete CI runtime-qualification gap without changing installed package behavior.
  • add merge-risk: 🚨 automation: Below-floor FreeBSD packages now stop the shared native workflow and its release prebuild consumer; refusing such proof is explicitly accepted in the maintainer-authored direction.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

  • P2: This closes a concrete CI runtime-qualification gap without changing installed package behavior.
  • merge-risk: 🚨 automation: Below-floor FreeBSD packages now stop the shared native workflow and its release prebuild consumer; refusing such proof is explicitly accepted in the maintainer-authored direction.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 26, 2026, 3:32 AM ET / 07:32 UTC.

@steipete

Copy link
Copy Markdown
Contributor

Confirmed the runtime-admission failures in run 40285117746: both FreeBSD architectures selected Node 22.23.2 / 24.21.0, below the official 22.23.4 / 24.22.1 floors. The new guard behaves correctly; these are missing patched-runtime qualifications, not flakes to rerun.

At head b5d97a4, the local JavaScript check pipeline passed with 108 tests and 14 platform-native skips, and independent P0–P2 review completed without findings. This does not replace FreeBSD native proof. Holding the PR open until patched pkg runtimes or a qualified replacement build route can make all four native jobs pass; no gate was bypassed.

@steipete steipete added the blocked Waiting on an external dependency label Oct 27, 2026
@steipete

Copy link
Copy Markdown
Contributor

Blocked until the FreeBSD native jobs are green.

The guard here is correct: FreeBSD's pkg currently installs Node 22.23.2 / 24.21.0, below the patched floors this PR enforces. The FreeBSD ports tree picked up 22.23.4 / 24.22.1 on 2026-10-24/25 (including the 2026Q4 branch), but the prebuilt FreeBSD 14 quarterly packages were last built on 2026-10-23. Once the next package build lands, we'll rerun the FreeBSD jobs and merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blocked Waiting on an external dependency merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants