Skip to content

refactor: run the native-runner CI suite in its own Rust-cached General tests lane - #13795

Open
devinfoley wants to merge 1 commit into
masterfrom
ci/native-runner-dedicated-lane
Open

devinfoley wants to merge 1 commit into
masterfrom
ci/native-runner-dedicated-lane

Conversation

@devinfoley

Copy link
Copy Markdown
Contributor

Thinking Path

  • Paperclip is the open source app people use to manage AI agents for work.
  • PR CI shards the server vitest suite across runners because the server project pins maxWorkers to 1.
  • One server suite (native-codex-runner.integration.test.ts) cargo-builds the Runner release binaries in beforeAll; in a plain shard with no Rust cache that is a ~4m30s cold compile per run.
  • fix: run the cargo-building native-runner CI suite in the Rust-cached vitest lane #13586 moved that suite into the Rust-cached Verify Paperclip Runner (vitest 2/2) lane, but only through script-level indirection — a GITHUB_WORKFLOW == "PR" name check plus a package-script wrapper that piggybacks the suite onto the final vitest shard — because the CI credentials that authored it could not push workflow-file edits.
  • This PR is the workflow-lane version that fix: run the cargo-building native-runner CI suite in the Rust-cached vitest lane #13586 explicitly deferred: the suite gets its own Rust-cached matrix lane in pr-trusted.yml, and the indirection is deleted.
  • The benefit is a simpler, explicit contract: the workflow declares the dedicated lane instead of scripts inferring it from a workflow name, the Verify Paperclip Runner vitest lanes go back to doing exactly one job, and the wrapper script disappears.

Linked Issues or Issue Description

Refs #13586 (the script-level workaround this replaces). Refs #13736 (the sibling cleanup in this series that also required workflow-edit permissions).

What Changed

  • .github/workflows/pr-trusted.yml: the general_tests matrix gains a native-runner entry that runs --group general-server-native-runner as its own lane. Two new steps, gated on the entry's rust_cache: true flag, restore the shared release-runner-v1 Rust cache with the same pinned-toolchain block and restore-only contract as Verify Paperclip Runner (the block is copied verbatim; reader and writer must agree on the key). The job declares NATIVE_RUNNER_SUITE_LANE: dedicated so the shard script leaves the suite out of the twelve server shards.
  • scripts/run-vitest-stable.mjs: the without-chat exclusion now keys on the explicit NATIVE_RUNNER_SUITE_LANE=dedicated declaration instead of sniffing GITHUB_WORKFLOW == "PR". Callers that declare nothing — local runs, release-verify.yml under the Release and Cloud readiness workflows — keep the suite in their server shards, exactly as before.
  • packages/paperclip-runner/package.json: test:typescript:vitest returns to the plain ensure:eval-build-deps && build:rust && vitest run chain (its pre-fix: run the cargo-building native-runner CI suite in the Rust-cached vitest lane #13586 value). scripts/run-pr-vitest-lane.mjs, the wrapper that appended the suite to the final PR vitest shard, is deleted.
  • scripts/__tests__/run-vitest-stable-shard.test.mjs: the contract pins move to the new wiring — the dedicated-lane declaration appears exactly once in pr-trusted.yml, exactly one matrix entry runs the group and it opts into the Rust cache, both Rust steps are gated on the flag, release-verify.yml declares no lane it does not provide, and the runner package script is the plain chain. The wrapper tests are gone with the wrapper. Coverage partitions (12 PR shards + chat + native-runner = full server group; 10 release shards keep the suite) are pinned as before.

Verification

  • node --test ./scripts/__tests__/run-vitest-stable-shard.test.mjs: all 23 tests pass locally, including the partition-coverage and new wiring pins.
  • actionlint .github/workflows/pr-trusted.yml passes.
  • This PR's own CI cannot exercise the new lane: pr.yml calls pr-trusted.yml@master, so this run uses the current workflow definition with this branch's scripts. That combination takes the designed fallback path — no lane is declared, so the suite runs inside a server shard (cold compile, slower but fully covered). Expect one slow General tests (server N/12) check on this PR only.
  • The dedicated lane activates for every PR on merge to master. The first post-merge PR runs should show General tests (native-runner) at roughly the setup cost plus an incremental cargo rebuild, and Verify Paperclip Runner (vitest 2/2) back to its pre-piggyback time (~2min vs ~3min today).

Risks

  • Merge-ref skew, new workflow + older tree: a stale merge ref that predates this PR still contains the wrapper and the GITHUB_WORKFLOW check. The new workflow's env declaration is ignored by the old script, which still excludes the suite via the name check, and the dedicated lane's group has existed since fix: run the cargo-building native-runner CI suite in the Rust-cached vitest lane #13586 — so the suite runs in the dedicated lane and, redundantly, on the final vitest shard. Duplicated coverage, never lost coverage, and only transient.
  • This PR's own run and any old-workflow + new-tree pairing: the suite stays in a server shard — the pre-fix: run the cargo-building native-runner CI suite in the Rust-cached vitest lane #13586 slower-but-covered behavior.
  • Cache miss on the new lane: release-runner-v1 restore is read-only and keyed identically to the existing consumers; a miss is a slow lane (cold compile within the 20-minute timeout, as every pre-fix: run the cargo-building native-runner CI suite in the Rust-cached vitest lane #13586 run demonstrated), never a wrong result.
  • A third copy of the toolchain-pin block now exists in pr-trusted.yml. The pin tests assert both new steps stay gated on rust_cache and the release-runner-v1 shared key stays in use, which catches the likeliest drift (deleting or unkeying the restore) but not every divergence.

Model Used

Claude Fable 5 (claude-fable-5), Claude Code session with workflow-edit permissions.

Checklist

  • I have included a thinking path that traces from project context to this change
  • I have specified the model used (with version and capability details)
  • I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
  • I have searched GitHub for duplicate or related PRs and linked them above
  • I have either (a) linked existing issues with Fixes: # / Closes # / Refs # OR (b) described the issue in-PR following the relevant issue template
  • I have not referenced internal/instance-local Paperclip issues or links (only public GitHub #NNN / github.com/paperclipai/paperclip URLs)
  • My branch name describes the change (e.g. docs/..., fix/...) and contains no internal Paperclip ticket id or instance-derived details
  • I have run tests locally and they pass
  • I have added or updated tests where applicable
  • I have updated relevant documentation to reflect my changes
  • I have considered and documented any risks above
  • All Paperclip CI gates are green
  • Greptile is 5/5 with no open P2s, recommendations, or follow-ups
  • I will address all Greptile and reviewer comments before requesting merge

@commitperclip

commitperclip Bot commented Sep 22, 2026

Copy link
Copy Markdown

✅ All checks passing — ready for Greptile review and maintainer approval.

— commitperclip

@greptile-apps

greptile-apps Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge, with no outstanding findings or new issues since the previous review.

Summary

This PR moves the cargo-dependent native-runner integration suite into a dedicated Rust-cached General tests matrix lane.

  • Declares the dedicated lane explicitly and excludes the suite from ordinary PR server shards.
  • Restores the shared Runner Rust cache read-only for the new lane.
  • Removes the previous workflow-name-based wrapper and restores the runner package’s plain Vitest command.
  • Updates shard and workflow contract tests for the new arrangement.
  • No code changed since the previous review, and the previous cache-assertion finding was manually resolved.

Reviews (2) · Last reviewed commit: "refactor: run the native-runner CI suite..."

Comment thread scripts/__tests__/run-vitest-stable-shard.test.mjs
@devinfoley devinfoley changed the title ci: run the native-runner suite in a dedicated Rust-cached General tests lane refactor: run the native-runner CI suite in its own Rust-cached General tests lane Sep 22, 2026
@devinfoley
devinfoley force-pushed the ci/native-runner-dedicated-lane branch from 98c8bc2 to 07bfd64 Compare September 22, 2026 03:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant