ci: extract shared self-hosted setup into composite actions - #110
ci: extract shared self-hosted setup into composite actions#110luohuan19 wants to merge 1 commit into
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR adds reusable composite actions for CI environment setup and orphaned task cleanup, then restructures the workflow into separate Qwen3 and DeepSeek jobs with model-directory validation, longer task timeouts, isolated caches, and post-run cleanup. ChangesCI infrastructure
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 137-138: Add a top-level permissions block near the workflow
header granting only contents: read, so both unit-tests-qwen and
unit-tests-deepseek use least-privilege GITHUB_TOKEN access while retaining
checkout and build functionality.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: dcce0ad8-68eb-41bb-a5f9-480a4c824690
📒 Files selected for processing (3)
.github/actions/kill-orphaned-tasks/action.yml.github/actions/setup-ci-job/action.yml.github/workflows/ci.yml
| unit-tests-deepseek: | ||
| runs-on: [self-hosted, linux, arm64, npu] |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Add an explicit least-privilege permissions: block. The workflow relies on the repository's default GITHUB_TOKEN permissions for both new self-hosted jobs, which zizmor flags as an overly broad posture. These jobs only checkout and build, so scope the token down — ideally a top-level permissions: block (applies to unit-tests-qwen too), or per-job blocks.
🔒️ Suggested top-level block (add near the top of the workflow)
permissions:
contents: read🧰 Tools
🪛 zizmor (1.26.1)
[warning] 137-214: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 137 - 138, Add a top-level permissions
block near the workflow header granting only contents: read, so both
unit-tests-qwen and unit-tests-deepseek use least-privilege GITHUB_TOKEN access
while retaining checkout and build functionality.
Source: Linters/SAST tools
c763ab7 to
9c98e78
Compare
The two self-hosted device jobs duplicated ~130 lines of setup (checkout,
pypto source sync, toolchain pins, ptoas/pto-isa install, conda venv +
activate.sh, build) and every host path was hard-coded in git. Pull all of
it into .github/actions/setup-ci-job, and the task-submit cleanup into
.github/actions/kill-orphaned-tasks.
Host layout now comes from the runner's .env (CONDA_ROOT, CI_CACHE_ROOT,
CANN_ROOT, PYPTO_{QWEN3,DSV4}_MODEL_DIR) instead of absolute paths in the
workflow, so a differently-laid-out host needs no repo change. Missing or
malformed config fails in the first step with an actionable message rather
than as an opaque mid-build error 40 minutes in.
Also in this change:
- Split unit-tests into unit-tests-qwen and unit-tests-deepseek so the
8-card DeepSeek borrow schedules independently of the single-card qwen3
runs. Each job takes its own pip cache dir and its own pypto build tree
(ci-cache/pypto-src-<namespace>-<runner>) — a shared tree let concurrent
syncs reset --hard / clean -ffdx under each other's in-flight configure.
- Kill orphaned tasks via `task-submit --find` instead of `--list`: --list
truncates the command column to 77 chars, so the old grep never matched
and the cleanup was silently a no-op.
- Retry transient git clone/fetch TLS drops with backoff, and retry the
from-source pypto build after purging its build tree.
- Detect the ptoas arch from `uname -m` (with the matching PTOAS_SHA256_<ARCH>)
instead of pinning aarch64; download to a mktemp file so concurrent
cache-miss jobs cannot clobber each other.
- Fall back to a clean re-clone when the incremental checkout fails on a
stale submodule object store.
- Raise task-submit --timeout from 1200s to 3600s; the accuracy guards were
being killed before finishing on a loaded host.
9c98e78 to
edeb760
Compare
Background
The two self-hosted device jobs each carried ~130 lines of identical setup — checkout, pypto source sync, toolchain pin resolution, ptoas/pto-isa install, conda venv +
activate.sh, from-source build — and every host path was hard-coded in the workflow, so a host with a different layout required a repo change.Changes
Two composite actions
.github/actions/setup-ci-job— shared setup for every self-hosted device job.github/actions/kill-orphaned-tasks— task-submit orphan cleanupHost layout moves to the runner's
.env(CONDA_ROOT,CI_CACHE_ROOT,CANN_ROOT,PYPTO_QWEN3_MODEL_DIR,PYPTO_DSV4_MODEL_DIR). No absolute path lives in git any more. Missing or malformed config (e.g. an inline comment in.env, which the runner does not strip) fails in the first step with an actionable message instead of surfacing 40 minutes later as an opaque mid-build error.Job split:
unit-testsbecomesunit-tests-qwenandunit-tests-deepseek, so the 8-card DeepSeek borrow schedules independently of the single-card qwen3 runs. Each job takes its own pip cache dir and its own pypto build tree (ci-cache/pypto-src-<namespace>-<runner>) — sharing one tree let concurrentreset --hard/clean -ffdx/rm -rf buildrun under the other job's in-flight libbacktrace configure, surfacing as intermittent "C compiler cannot create executables".Fixes carried along
task-submit --findinstead of--list.--listis a human-facing view that truncates the command column to 77 chars, so the old grep for the full checkout path never matched and the cleanup was a silent no-op. Requires task-submit >= 2026-07-14 on the runner.uname -m(with the matchingPTOAS_SHA256_<ARCH>) instead of pinning aarch64; download to amktempfile so two concurrent cache-miss jobs cannot clobber each other.task-submit --timeoutfrom 1200s to 3600s; the accuracy guards were being killed before finishing on a loaded host. The DeepSeek job keeps--max-time 2700from feat(serving): report OpenAI-style token usage in HTTP responses #107.Deployment requirement
Before merging, each self-hosted runner needs the variables above added to
<runner-root>/.env(one bareKEY=VALUEper line, no quotes and no trailing comments) and the runner service restarted. task-submit on the runner also has to be new enough to support--find.