Skip to content

fix(deps): preflight runtime dependencies - #2557

Merged
kongche-jbw merged 1 commit into
alibaba:mainfrom
kongche-jbw:fix/sec-core/preflight-runtime-deps
Aug 20, 2026
Merged

fix(deps): preflight runtime dependencies#2557
kongche-jbw merged 1 commit into
alibaba:mainfrom
kongche-jbw:fix/sec-core/preflight-runtime-deps

Conversation

@kongche-jbw

@kongche-jbw kongche-jbw commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Why

build-all.sh could build or begin installation before discovering missing runtime
dependencies such as bwrap. It also checked only sec-core, so users could hit
the same one-at-a-time failure pattern in other selected components.

What changed

  • Read runtime dependencies from every selected component manifest by TOML key
    and report all missing capabilities together.
  • Adapt packaged contracts for source builds, including excluding sec-core systemd
    and mapping source-specific Node and OpenSSL requirements.
  • In user mode, check platform blockers first, preserve user-local source setup
    such as nvm, then re-probe every runtime and report remaining gaps once.
  • In system mode, reject platform and language-runtime blockers before package
    mutation, refresh APT indexes for DEB hosts, install native packages in one
    deduplicated transaction, and re-probe.
  • Require Node >=20 on the fixed system PATH for system installs; an installing
    user nvm never satisfies the system contract, and no external repository is added.
  • Preserve user PATH priority in user mode while adding standard system directories
    for host tools such as mkfs.btrfs and modprobe.
  • Skip ws-ckpt runtime probes in user mode because its user install profile is a no-op.
  • Restore --ignore-deps as an explicit bypass for dependency setup and runtime
    verification on pre-provisioned hosts.
  • Default uv Python standalone downloads to the official source, honor explicit
    mirror overrides, and migrate only legacy configuration managed by build-all.
  • Restore any pre-existing system Node path after source-build CI completes.
  • Emit absolute retry commands and preserve --deps-only and --ignore-deps.

Related issue

Related to #2520. The direct linux-sandbox diagnostic remains separate.

User / Agent impact

Default user builds retain automatic Node setup through nvm. Missing runtime
dependencies are reported together before component files are installed. System
installs require language runtimes to be present in the fixed system PATH;
ordinary native packages may be installed automatically after all blockers pass.
--ignore-deps deliberately bypasses both setup and verification when callers
have already provisioned the host.

Risk and compatibility

  • Public CLI, API, configuration, or documented behavior changed
  • Privileged or security-sensitive behavior changed
  • Cross-component contract changed

System mode may refresh APT metadata and invoke the detected package manager once
for missing native runtime packages, but it never configures NodeSource or another
external repository. User mode never invokes privileged runtime package
installation; its existing user-local source dependency installers remain available.

Validation

  • bash -n scripts/build-all.sh tests/test-build-all-runtime-deps.sh
  • ./tests/test-build-all-runtime-deps.sh (34/34)
  • cargo test -p anolisa-core manifest --locked (93 passed)
  • Workflow YAML parsed with PyYAML
  • Reordered TOML fields and reversed deb / rpm inline-table keys
  • sec-core source dependencies exclude the packaged systemd requirement
  • Source-build CI no longer preinstalls or verifies systemd
  • APT refresh precedes the aggregated DEB install and failure stops before mutation
  • User deps-only Node setup runs before the full runtime re-probe
  • User-local nvm provisioning selects Node 24 and remains ahead of system PATH
  • System Node 18 plus current-shell/nvm Node 24 is rejected
  • Language runtimes never enter the system package transaction
  • Restricted user PATH discovers loadable btrfs and system mkfs.btrfs
  • ws-ckpt rejects a btrfs-progs probe when mkfs.btrfs is unavailable
  • User installs omit ws-ckpt daemon runtime and platform requirements
  • --ignore-deps skips install-time runtime preflight without blocking installation
  • Absolute retry commands preserve deps-only and ignore-deps modes
  • User and system cosh deps-only dry runs
  • build-all and CI use the same official uv Python standalone default
  • Legacy build-all-managed Nanjing mirror configuration migrates to official
  • Explicit uv mirror overrides and user-owned uv configuration are preserved
  • CI failure paths print the final 200 lines of target/build.log
  • CI restores the pre-existing /usr/local/bin/node on success and failure
  • git diff --check

Documentation and rollback

Updated the bilingual source-build guide, sec-core README, and user guide with
runtime preflight behavior, the system Node requirement, Bash 4.3 minimum,
the uv Python mirror override, and --ignore-deps responsibility. Revert commit f3909e98 to restore the previous
component-specific behavior.

@kongche-jbw
kongche-jbw force-pushed the fix/sec-core/preflight-runtime-deps branch from 37bdd24 to 796dd4d Compare August 14, 2026 10:12
@github-actions github-actions Bot added scope:ci ./.github/ scope:scripts ./scripts/ labels Aug 14, 2026
@kongche-jbw
kongche-jbw force-pushed the fix/sec-core/preflight-runtime-deps branch from 796dd4d to b437ae7 Compare August 14, 2026 10:42
@kongche-jbw kongche-jbw changed the title fix(sec-core): preflight runtime deps fix(deps): preflight runtime dependencies Aug 14, 2026
@kongche-jbw
kongche-jbw force-pushed the fix/sec-core/preflight-runtime-deps branch 8 times, most recently from 95e98b9 to 05f48a5 Compare August 14, 2026 23:56
@kongche-jbw
kongche-jbw marked this pull request as ready for review August 15, 2026 02:03
@kongche-jbw

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 05f48a5ee2

ℹ️ 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".

Comment thread scripts/build-all.sh
Comment thread scripts/build-all.sh
@kongche-jbw
kongche-jbw force-pushed the fix/sec-core/preflight-runtime-deps branch 2 times, most recently from 899421a to 35746f1 Compare August 15, 2026 02:22

@Forrest-ly Forrest-ly left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review for commit 35746f1 — approving.

What I verified

  • Ran tests/test-build-all-runtime-deps.sh locally against this commit: 28/28 pass.
  • Ran runtime_dependencies_for_manifest against all 8 real component manifests: emits exactly the expected 15 records; agent-memory and os-skills correctly contribute zero.
  • Preflight ordering confirmed in code and tests: platform capabilities are checked before user source setup; system mode blocks platform-capability / language-runtime blockers before any package mutation; one deduplicated APT transaction preceded by apt-get update (failure stops before mutation); re-probe after install; install phase re-verifies before any component file lands.
  • The two earlier automated findings (sec-core systemd exclusion for source builds, APT refresh before the aggregated install) are both addressed in this commit with regression tests.
  • User mode never calls as_root for runtime packages; system probes use the fixed RUNTIME_SYSTEM_PATH, so a user-local nvm Node cannot satisfy the system contract; the emitted retry command is absolute and preserves --system / --deps-only / --ignore-deps (covered for --component memory).
  • agent-memory code is untouched; it participates via the generic manifest-driven preflight (it declares no dependencies), so no new requirements are introduced for memory installs.
  • The ws-ckpt probe change (btrfs versionmkfs.btrfs --version) matches what the removed installer actually required (cmd_exists mkfs.btrfs).

Non-blocking suggestions

  1. scripts/build-all.sh:2128,2150 — the local -n namerefs are the script's first bash ≥ 4.3 dependency. On bash 4.2 hosts (CentOS/Anolis 7 era) the preflight would error out instead of reporting. Likely acceptable given the ws-ckpt/sight kernel floors and EL7 EOL, but worth confirming EL7 is out of support scope, or documenting a minimum bash version.
  2. User-visible behavior change worth a release note: missing runtime dependencies now hard-fail user-mode installs, where bwrap/gpg/jq previously only warned. Intentional per the PR body, but users will notice.
  3. The awk TOML reader assumes single-line packages = { ... } inline tables and no | character in any field. Current manifests comply and the 15-record assertion guards against drift, but consider a one-line comment near runtime_dependencies_for_manifest spelling out these constraints so future manifest edits don't silently drop records.

Nice work on the aggregated reporting, the strict order-of-operations guarantees, and the test coverage.

@kongche-jbw

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 35746f1d87

ℹ️ 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".

Comment thread scripts/build-all.sh
@kongche-jbw
kongche-jbw force-pushed the fix/sec-core/preflight-runtime-deps branch from 35746f1 to a463f7b Compare August 17, 2026 02:29

@Forrest-ly Forrest-ly left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review for commit a463f7bf (re-review after force-push squash) — approving.

What I verified

P1 fix (skip runtime checks for user-ineligible components). The delta vs the previous head 35746f1d is exactly the fix plus one regression test: a new runtime_install_components() filter (user mode drops ws-ckpt) now feeds both selected_runtime_dependencies and source_build_runtime_dependencies. I confirmed this is correct:

  • src/ws-ckpt/Makefile makes INSTALL_PROFILE=user install a genuine no-op ("User mode: ws-ckpt requires a systemd/root daemon; skipping install."), and its manifest declares [component.layout] modes = ["system"], so its daemon runtime/btrfs requirements are irrelevant in user mode.
  • Ran the real (non-stubbed) collection logic on a simulated btrfs-less host (empty /proc/filesystems entry + empty RUNTIME_SYSTEM_PATH so modprobe is unavailable): user mode with default components collects 0 platform-capability gaps (previously aborted before any dependency setup), while system mode with --component ws-ckpt still correctly reports ws-ckpt|btrfs [platform-capability] as missing.
  • System mode with default components retains all three ws-ckpt records (btrfs-progs with the mkfs.btrfs --version probe, rsync, btrfs platform capability).
  • The filter is correctly scoped: sight is the only other mode-restricted component, but its user profile genuinely installs files (user PREFIX, INSTALL_SYSTEMD=0), so it rightly remains unfiltered.

Tests. Ran tests/test-build-all-runtime-deps.sh against this commit locally: 29/29 pass, including the new test_user_skips_ws_ckpt_noop_install_dependencies covering both directions (user default omits ws-ckpt; explicit system ws-ckpt keeps it).

Earlier P2s remain satisfied in the squashed diff. sec-core systemd is excluded from source-build dependencies (runtime_dependency_for_source_build), and DEB system preflight refreshes APT indexes before the single deduplicated transaction with a hard stop on refresh failure. Both retain their regression assertions.

Full squashed single-commit diff reviewed (all 5 files, +1300/-55): manifest-driven awk parser with key-based (not order-based) reads, source-build adapters (openssl1.1openssl, sec-core nodejsnode >=20), two-phase user preflight (platform-only → source setup → full), system preflight blocking platform/language-runtime blockers before any package mutation, fixed RUNTIME_SYSTEM_PATH contract for system installs (nvm can't satisfy it), absolute retry commands preserving --system/--deps-only/--ignore-deps, and the guarded main enabling the test harness to source the script. CI changes are consistent: the sec-core source-build jobs preinstall the runtime contract (or let system-mode preflight install it on a clean apt state, which exercises the new aggregated path), and the new check-build-runtime-deps job wires the test suite into CI. agent-memory participates via its manifest, which declares no dependencies — no new requirements for memory installs.

Non-blocking notes (no action required)

  1. The user-mode ws-ckpt exemption is hardcoded; if more mode-restricted components appear, deriving the filter from [component.layout] modes would generalize it. Current approach is explicit, commented, and test-covered, so this is fine for now.
  2. Carry-over from the previous review, still present and still acceptable: local -n namerefs require bash >= 4.3, and missing system packages now hard-fail user-mode installs (previously warn-only). Both were already acknowledged.

@kongche-jbw
kongche-jbw force-pushed the fix/sec-core/preflight-runtime-deps branch 2 times, most recently from ecdcf96 to 70d6507 Compare August 17, 2026 03:58

@Forrest-ly Forrest-ly left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review 通过(re-review @ 70d6507)。

对比上一轮已批准的 head a463f7b,70d6507 的变更仅限 .github/workflows/sec-core-source-code-build.yaml(+17/-2),两处 CI 改进均正确:

  1. UV_PYTHON_INSTALL_MIRROR 指向官方 astral-sh python-build-standalone 源:scripts/build-all.sh_configure_uv_mirror 使用 ${UV_PYTHON_INSTALL_MIRROR:-<nju mirror>},workflow 级 env 在 CI 中正确生效,本地构建默认值不受影响,与 PR body 描述一致。
  2. 构建失败时打印 target/build.log 最后 200 行(::group:: 折叠)并保留原始退出码:日志路径与 LOG_FILE="$OUTPUT_DIR/build.log"OUTPUT_DIR=$PROJECT_ROOT/target)一致;在 GitHub Actions 默认 bash -e 下逐句验证(|| build_status=$? 捕获、if (( ... )) 条件上下文、tail ... || echo 兜底、exit "$build_status"),成功/失败两条路径行为均正确,用户模式成功后追加 $GITHUB_PATH 的顺序也保留。

同时确认:

  • scripts/build-all.sh、manifests、tests 在两个 head 之间无任何变更,上轮验证过的逻辑原样保留;bot 的 P1 修复(user 模式跳过不安装文件的 ws-ckpt 的 runtime 依赖收集)在最终 head 仍成立。
  • 本地回归 ./tests/test-build-all-runtime-deps.sh70d6507 上 29/29 通过;YAML 解析与 bash -n 均通过。
  • CI(70d6507):Source Build (Ubuntu 22.04 / Alinux4)、Source Build System Install、Check build runtime dependencies、Test anolisa 等全部 SUCCESS,无 failure。

代码质量良好,未发现新问题,approve。

@kongche-jbw
kongche-jbw force-pushed the fix/sec-core/preflight-runtime-deps branch from 70d6507 to 8561e1e Compare August 20, 2026 02:54

@ikunkun-sys ikunkun-sys left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the well-engineered preflight rewrite — the aggregated manifest-driven probing, user/system mode split, fail-closed version checks, and the 29-case test suite were all verified locally (29/29 shell tests, 93 anolisa-core manifest tests reproduced). A few contract regressions need attention before merge:

P1 — Must Fix

1. --ignore-deps contract regression: the flag can no longer skip runtime dependency enforcement, and help/docs were not updated

  • do_install() now unconditionally runs preflight_runtime_dependencies || return 1 (L2465-L2472) with no $INSTALL_DEPS gate.
  • Inside the preflight, the [[ "$INSTALL_MODE" != "system" ]] || ! $INSTALL_DEPS branch (L2270-L2274) reports and returns 1 for user mode or --ignore-deps.
  • Baseline behavior: --ignore-deps = "build + install, skip dep install" (help L8/L2776/L2795/L2804, AGENTS.md, docs/BUILDING_zh.md), and missing bwrap/gpg/jq in user mode was warn-only.

Net effect: ./scripts/build-all.sh --ignore-deps --component sec-core (the exact usage documented in the help at L2804) on a minimal container without jq/bwrap now completes the entire build first, then fails at install time — and no flag remains to bypass the runtime dependency check (--no-install doesn't install at all). The PR body states "No documentation changes are required", but the flag semantics are inverted relative to the help/docs.

Please either gate the preflight on $INSTALL_DEPS and downgrade to an aggregated warning under --ignore-deps, or keep the strict behavior but update the help (L8/L2776/L2795/L2804), AGENTS.md, and BUILDING*.md to state that runtime dependencies are still verified and must be present.

P2 — Should Fix

2. System-mode Node repository-install path is now unreachable dead code; documented --system usage starts failing on hosts it used to work on

  • do_install_deps() runs the full system-mode preflight before install_node (L1458-L1464); sec-core's nodejs dependency is rewritten as a language-runtime >= 20 blocker, so a host without Node aborts immediately.
  • install_node()'s repository-install branch and its die "Repository Node.js cannot satisfy ..." (L655-L662) can therefore never execute in the normal flow — only the unit tests reach it.
  • A host whose distro repo provides Node >= 20 but where Node isn't installed yet (previously: dnf install nodejs npm → success) now fails per the documented --system usage (docs/BUILDING_zh.md). CI compensates by preinstalling Node 24 + symlink; local users cannot.

Consider letting the system-mode preflight attempt install_node's repository path once before blocking, and document the Node >= 20 precondition for --system in BUILDING*.md / QUICKSTART.

3. User mode tightened from warn-and-continue to hard failure without doc updates

Missing bubblewrap/gnupg/jq in user mode now aborts (L1494-L1497 + L2465-L2470) instead of warn-only. The fail-fast intent is sound, but the QUICKSTART path (./scripts/build-all.sh --component sec-core, docs/user-guide//agent-sec-core/QUICKSTART.md) and BUILDING.md should list the runtime prerequisites (bubblewrap, gnupg, jq, python3, pkg-config + openssl-devel), or provide a --skip-runtime-check escape hatch (could share the mechanism with item 1).

4. Self-hosted runner global state: /usr/local/bin/node symlink persists across jobs

Both jobs in .github/workflows/sec-core-source-code-build.yaml run sudo ln -sfn "$(command -v node)" /usr/local/bin/node (L52-L55, L133-L136) with no cleanup. If the anolisa-k8s-general-ci-x64 pool reuses pods, the symlink leaks into other workflows' assumptions about the system PATH, and future setup-node version changes silently rewrite it. Please add an if: always() cleanup step (sudo rm -f /usr/local/bin/node), or use a side-effect-free probe that expands $(command -v node) directly.

P3 — Minor (non-blocking)

  • The shell TOML parser only supports single-line inline tables (packages = { rpm = ..., deb = ... }); a future multi-line inline table or dotted key would silently yield empty package names while the Rust side parses fine — consider warning on an unclosed packages = {.
  • Unparseable language-runtime version: the shell side judges "missing" (L2058-L2063) while anolisa-core's version_verdict treats it as NotVerified → Resolved — verdicts would diverge if a non-semver runtime is ever added.
  • User-mode default flow runs the full preflight twice (do_install_deps tail L1495-L1497 and do_install L2469-L2470) — log noise only.
  • UV_PYTHON_INSTALL_MIRROR switched to GitHub direct while the rest of this pipeline uses China mirrors — worth watching this workflow's stability.
  • nvm fallback bumped to Node 24 while build-copilot-shell CI pins Node 20 — matrix divergence for a native-module-dependent package.
  • local -n raises the implicit bash requirement to >= 4.3 (fine for supported distros, but undeclared).
  • anolisa-core/src/manifest.rs doc comment still shows the old btrfs version probe example (the ws-ckpt probe change itself is verified safe for all downstream consumers).
  • Cosmetics: trailing spaces in the retry/install hint lines (printf '%q '); command -v gpg in the CI verify steps prints the path into logs (add >/dev/null).

Summary: completeness and implementation quality are excellent — all 10 "What changed" claims verified in code, and the manifest key contract is consistent between build-all.sh and anolisa-core. The blocker is the --ignore-deps semantics inversion plus the undocumented tightening of user/system mode failure behavior; the rest are documentation and CI hygiene items.

@kongche-jbw
kongche-jbw force-pushed the fix/sec-core/preflight-runtime-deps branch from 8561e1e to 1199984 Compare August 20, 2026 03:30
@kongche-jbw

Copy link
Copy Markdown
Collaborator Author

@ikunkun-sys Addressed the review in 11999844.

  • P1 --ignore-deps: restored the documented contract. The install phase now skips runtime preflight when dependency handling is disabled, emits an explicit warning, and continues installation. Added a regression test; the runtime-dependency suite is now 34/34.
  • System Node path: kept language runtimes manual by design, matching the CLI policy and avoiding package mutation before aggregate blockers are resolved. Removed the misleading system repository-install path from install_node; system mode now explicitly requires Node >=20 in the fixed system PATH. This contract is documented in both BUILDING guides.
  • User-mode prerequisites: documented the aggregate fail-before-install behavior and the Node/bubblewrap/GnuPG/jq requirements in the bilingual BUILDING guide, sec-core README, and sec-core Quickstart. The docs also clarify that --ignore-deps transfers runtime responsibility to the caller and declare Bash >=4.3.
  • CI global state: the temporary /usr/local/bin/node now backs up any previous entry, restores it in an if: always() step, and also rolls back immediately if setup fails. The noisy command -v gpg output was removed.

The remaining P3 parser/version suggestions do not affect any current manifest contract and are left out of this focused fix; the final user-mode preflight remains intentionally duplicated as the no-files-installed guard immediately before installation. Local validation passed: shell syntax, 34/34 runtime tests, workflow YAML parsing, and git diff --check.

@ikunkun-sys ikunkun-sys left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the thorough response — all four blocking findings from the previous round are verified as resolved (with tests and docs): the --ignore-deps bypass is fully restored end-to-end (preflight skipped, install proceeds), the system-mode Node requirement is documented with an actionable defensive die, the user-mode prerequisites are documented accurately against the actual probe set, and the CI node symlink is now backed up and restored on both success and failure. The re-run test suite (34/34) and workflow YAML validation were reproduced locally. One documentation gap remains before merge, plus a few non-blocking notes:

P2 — Should Fix

1. uv Python download source flipped to GitHub direct for all local builds, but the UV_PYTHON_INSTALL_MIRROR escape hatch is documented nowhere

_configure_uv_mirror() now defaults python-install-mirror to https://github.com/astral-sh/python-build-standalone/releases/download (previously the NJU mirror, which existed for restricted networks) and migrates build-all-managed legacy configs in place. However, a repo-wide search finds zero mentions of UV_PYTHON_INSTALL_MIRROR / python-install-mirror in docs/, READMEs, CONTRIBUTING, the website, --help, or usage() — users behind restricted networks (the original audience of the NJU default; the rest of the pipeline still defaults to aliyun PyPI / npmmirror) whose Python download now fails or stalls have no documented recovery path.

Suggested fix (~4 lines, bilingual, in the BUILDING.md / BUILDING_zh.md prerequisites section):

Python runtimes are downloaded from GitHub (astral/python-build-standalone) by
default; on restricted networks set `UV_PYTHON_INSTALL_MIRROR` to a reachable
mirror before building.

P3 — Non-blocking

  • --help (L2804) and AGENTS.md (L32) still describe --ignore-deps as "Skip dependency installation" only; the flag now also skips runtime dependency verification (documented in BUILDING.md, but --help is the first contract surface users see). A one-line update would close the gap: --ignore-deps Skip dependency installation and runtime dependency verification (pre-provisioned hosts only).
  • The CI expose step's trap restore_node ERR does not cover hard cancellation (SIGTERM) in the sub-second window between the sudo mv backup and the $GITHUB_ENV write, where the symlink could leak. Well-mitigated as is (if: always() restore runs on cancel, per-job pods, self-healing fixed-name backup), so purely optional hardening: trap restore_node ERR EXIT.
  • Remaining minor items from the previous round, acceptable to defer: unparseable-version semantics divergence between the shell preflight (missing) and anolisa-core's version_verdict (NotVerified → Resolved); single-line-only inline-table support in the shell TOML parser; stale btrfs version probe example in anolisa-core/src/manifest.rs doc comments; nvm Node 24 vs copilot-shell CI Node 20 matrix divergence; duplicate user-mode preflight log noise.

Everything else in the increment checks out: the do_install --ignore-deps gate restores the bypass semantics exactly (preflight call count 0, install proceeds, dry-run branch symmetric), the install_node die relocation leaves no reachable system-mode repository-install path, the uv legacy migration is marker-gated and fail-open (user-owned configs untouched), and the CI backup/restore handles dangling symlinks, in-step ERR, and cancellation. Happy to approve once the P2 doc addition lands.

- Aggregate manifests and block system changes before dependency setup.
- Preserve user-local source provisioning, then re-probe every runtime.
- Parse manifest keys safely and batch native system package installs.

Fixes: 6e4a741 ("refactor(build): introduce unified build workflow")
Signed-off-by: kongche-jbw <kongche.jbw@alibaba-inc.com>
@kongche-jbw
kongche-jbw force-pushed the fix/sec-core/preflight-runtime-deps branch from 1199984 to f3909e9 Compare August 20, 2026 06:20
@kongche-jbw

Copy link
Copy Markdown
Collaborator Author

@ikunkun-sys Fixed the remaining documentation gap in f3909e98.

  • Added bilingual BUILDING guidance that uv-managed Python runtimes default to the official astral-sh/python-build-standalone GitHub downloads and that restricted-network users can set UV_PYTHON_INSTALL_MIRROR to a compatible mirror base URL. Both languages include the same export example.
  • Updated the script header, --help, command examples, and AGENTS.md so --ignore-deps consistently means skipping dependency setup and runtime verification, for pre-provisioned hosts only.
  • Left the optional cancellation trap hardening unchanged; the existing in-step ERR rollback plus if: always() restore already covers the supported runner lifecycle.

Validation passed: bash -n, 34/34 runtime dependency tests, rendered --help assertions, bilingual docs lint/tree parity, relative-link checking, and git diff --check.

@ikunkun-sys ikunkun-sys left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All findings from the previous rounds are now resolved and verified — thanks for the thorough iterations:

  • The --ignore-deps bypass semantics are fully restored and consistently documented across --help, AGENTS.md, BUILDING, QUICKSTART, and the sec-core READMEs.
  • The UV_PYTHON_INSTALL_MIRROR escape hatch is now documented bilingually in BUILDING.md / BUILDING_zh.md (§3 toolchain section) with an example, matching the actual _configure_uv_mirror behavior point by point.
  • The CI node symlink backup/restore, the system-mode Node >= 20 requirement, and the Bash 4.3 minimum are all in place; the test suite (34/34) was reproduced locally across rounds, and the final force-push is a clean text-only increment (+22/-5) with no hidden logic changes.

One last non-blocking suggestion: CONTRIBUTING.md (L128) and CONTRIBUTING_zh.md (L106) still describe --ignore-deps as "Skip dependency installation / 跳过依赖安装" — the only two stale references left repo-wide after this PR's semantic change. A two-line follow-up (or a quick amend if it fits this PR's scope) would make the flag's documentation fully consistent:

| `--ignore-deps` | Skip dependency setup and runtime verification (pre-provisioned hosts only). |
| `--ignore-deps` | 跳过依赖安装与运行时校验(仅适用于已预装依赖的主机)。 |

Approving — great work on this PR. The preflight design (aggregated manifest-driven probing, user/system mode split, fail-closed version checks, marker-gated legacy migration) and the responsiveness across review rounds are both excellent.

@kongche-jbw
kongche-jbw merged commit 8403e88 into alibaba:main Aug 20, 2026
31 checks passed
@kongche-jbw
kongche-jbw deleted the fix/sec-core/preflight-runtime-deps branch August 20, 2026 07:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope:ci ./.github/ scope:scripts ./scripts/

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants