feat(workspace): add package manager torture lab - #35
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12cc81bc05
ℹ️ 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".
|
@codex review |
|
@codex security review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2458b56a8
ℹ️ 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".
| fn valid_identifier(value: &str) -> bool { | ||
| !value.is_empty() | ||
| && value | ||
| .bytes() | ||
| .all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'.' | b'_' | b'-')) |
There was a problem hiding this comment.
Reject . and .. explicitly rather than accepting every string composed of punctuation allowed here. For example, a configured channel and alias both named .. pass this predicate, after which sync_channels builds the destination with cache.join(channel_name).join(alias), escaping the per-channel cache tree and potentially overwriting another index or Sage cache file.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 191f969 修复：valid_identifier 现在显式拒绝 . 和 ..；回归覆盖 channel、subchannel 名和 alias 三个入口各自的两个 dot-segment。针对性测试、66 项主集成测试和完整 CI 均通过。
| let selected = solution.iter().find_map(|(key, version)| { | ||
| universe | ||
| .release(key, version) | ||
| .is_some_and(|package| package.provides.contains(&symbol)) | ||
| .then(|| key.clone()) |
There was a problem hiding this comment.
Derive this binding from the selected virtual proxy rather than the first concrete package in solution order. When two selected packages provide the same symbol—for example, one is an explicit root while another satisfies a constrained virtual dependency—this find_map can choose the former regardless of version constraints or provider preference, causing LMDB to record a different system provider from the one PubGrub actually used.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 191f969 修复：SageSolver 在过滤私有 __sage proxy 前，从已选 proxy release 的唯一 concrete dependency 提取实际 Provider；ReconcilePlan 直接使用该结果，不再按 solution 顺序猜测。新增“显式旧 Provider root + 受版本约束的实际 Provider”回归，针对性测试、全量测试和 CI 均通过。
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 191f969147
ℹ️ 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".
| let archive = obtain_release_archive(&engine, &package_cache, source).await?; | ||
| let inspection = sage_archive::inspect_package(&archive)?; |
There was a problem hiding this comment.
When an indexed package contains a structurally invalid payload, such as a hard link, missing indexed entry, or duplicate payload path, inspect_package succeeds because it stops at the first data/ entry. The journal is then written before extract_package_internal performs validate_payload, so extraction fails with no publication but leaves a pending journal that every later mutating command retries and fails on, including the sync needed to obtain a corrected index. Run full payload validation during this preflight, before write_journal.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 dd7c5a6 修复：安装 preflight 在创建 journal 前调用公开的完整 payload 无写入验证。新增恶意 hardlink 包回归，确认安装失败后 pending journals 和 package state 均为空，随后健康包仍可正常安装；全量测试和两条 CI 均通过。
| } else if entry.header().entry_type().is_file() { | ||
| if entry.size() != record.size { | ||
| return Err(ArchiveError::InvalidMetadata(format!( | ||
| "size mismatch for {}", | ||
| relative.display() | ||
| ))); | ||
| } |
There was a problem hiding this comment.
When a later regular entry has the declared size but contents that disagree with files.idx, this scan accepts it because it checks only entry.size(). Extraction can therefore atomically publish earlier entries before detecting the bad checksum while writing the later entry, leaving a partially upgraded root and a journal that cannot complete on retry. Consume and hash each regular entry during the no-write validation pass so all checksum failures occur before the first rename.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 dd7c5a6 修复：payload 预检对每个 regular entry 复用流式 verify_reader，提前校验 size 与 SHA-256。新增“首条有效、后置同长度坏哈希”回归，确认失败时首条文件也未发布；全量测试和两条 CI 均通过。
| if !seen.insert(relative.to_path_buf()) { | ||
| return Err(ArchiveError::InvalidMetadata(format!( | ||
| "duplicate payload {}", | ||
| relative.display() | ||
| ))); |
There was a problem hiding this comment.
When the index contains both a file or symlink path and one of its descendants, such as opt/app and opt/app/bin/tool, this validation only detects exact duplicates and accepts both. Extraction then publishes whichever entry appears first and fails when the other entry tries to treat the same path as the opposite filesystem type, leaving a partial package state despite the promised pre-write validation. Reject any indexed non-directory path that is an ancestor of another indexed path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 dd7c5a6 修复：完整 payload 验证在任何写入前检查所有 indexed path 的祖先，拒绝非目录索引项同时作为另一项的祖先。新增 opt/app 与 opt/app/bin/tool 回归，确认两者均未落盘；全量测试和两条 CI 均通过。
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd7c5a6cb1
ℹ️ 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".
| if relative.as_os_str().is_empty() || entry.header().entry_type().is_dir() { | ||
| continue; |
There was a problem hiding this comment.
When an archive includes an extra directory such as data/var/unowned/, this branch skips it during preflight, but extraction later creates it with ensure_directory even though it is absent from files.idx and package ownership. A successful install therefore leaves unmanaged filesystem debris that removal cannot clean up; a directory colliding with an indexed file can instead make extraction fail after earlier files were published. The fresh case not covered by the indexed-ancestor fix is that directory headers never enter either collision or membership validation. Validate directories against the implied ancestor set of indexed paths before allowing them.
AGENTS.md reference: AGENTS.md:L77-L77
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 72b3e97 修复：payload validator 从 indexed leaf 构造必要祖先目录集合，目录 header 只有属于该集合才允许；额外空目录及与 indexed leaf 同路径的目录均在首次写入前拒绝。新增两类零落盘回归，完整测试和两条 CI 均通过。
| 0o755 | ||
| }), | ||
| ) { | ||
| Ok(()) | Err(Errno::EEXIST) => {} |
There was a problem hiding this comment.
When the final lock directory already exists with permissions allowing another user to rename entries, accepting EEXIST without checking its owner or mode leaves the host lock replaceable: that user can unlink operation.lock while one process holds its inode, after which another Sage process creates and locks a different inode and both writers proceed concurrently. This can occur with a pre-created or misconfigured /run/sage; verify and repair the directory's ownership and permissions before opening the lock file.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 72b3e97 修复：最终专属锁目录经 anchored openat(O_NOFOLLOW) 打开后，在创建 operation.lock 前用 fstat/fchown/fchmod 修复到 effective owner 与 0700，并二次验证；非 root 遇到外来 owner 时安全失败。新增预建 0777 目录回归，完整测试和两条 CI 均通过。
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72b3e9778c
ℹ️ 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".
| - Production Rust source size is informational for this release; correctness | ||
| gates are not weakened to meet the historical 9,000-line target. |
There was a problem hiding this comment.
Do not demote the source-size limit to an informational metric: this commit adds 3,215 net Rust lines and leaves the tracked workspace at 14,713 lines, despite the repository's strict whole-workspace maximum of 9,000. The newly added torture code is part of the workspace, so it must be reduced or consolidated rather than documenting the mandatory gate as waived.
AGENTS.md reference: AGENTS.md:L24-L26
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 470698dd1f
ℹ️ 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".
| if let Some(previous) = bindings.insert(symbol.clone(), key.clone()) { | ||
| if previous != key { | ||
| return Err(SolverError::Internal(format!( | ||
| "virtual symbol {symbol} selected both {previous} and {key}" | ||
| ))); |
There was a problem hiding this comment.
When two desired roots impose non-overlapping constraints on the same virtual symbol and coinstallable packages satisfy each constraint, the initial apply_packages call in rebuild_system uses resolve() and publishes both providers, but this later binding pass detects the two keys and returns an internal error. The rebuild therefore fails only after changing the filesystem and LMDB, before pruning, service rendering, and rebuild triggers; perform this binding validation during the initial solve or otherwise before publishing packages.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 1c72c7e 修复：rebuild 现在先加载 available/installed 并执行带 Provider binding 的完整 ReconcilePlan 求解，只有成功后才进入 apply_packages，因此同一接口出现不兼容 Provider 选择时不会先修改文件系统或 LMDB。新增双 root 非重叠 virtual/libc 约束回归；全量测试和两条 CI 均通过。
| let provider_bindings = preferences | ||
| .into_iter() | ||
| .map(|(symbol, key)| (symbol.strip_prefix("virtual/").unwrap_or(&symbol).into(), key)) | ||
| .into_keys() | ||
| .filter_map(|symbol| { | ||
| let selected = selected_providers.get(&symbol)?.clone(); |
There was a problem hiding this comment.
When a configured interface is no longer referenced by any desired package, selected_providers has no entry and this filter_map silently omits it. rebuild_system only calls set_system_provider for returned entries and has no deletion pass, so a previous binding remains in LMDB even when reconciliation prunes its provider package; emit an explicit removal or replace the complete binding set during reconciliation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 1c72c7e 修复：DB API 改为单事务 replace_system_providers，先清空旧集合再写入本次完整 binding；不再被选择的接口会被删除。新增写入 libc binding 后以空集合替换的回归；全量测试和两条 CI 均通过。
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c72c7e97f
ℹ️ 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".
| let releases = final_paths | ||
| .get(&owner) | ||
| .is_some_and(|paths| !paths.contains(path)); |
There was a problem hiding this comment.
When package A releases a modified configuration path such as /etc/app.conf and package B claims it in the same upgrade, this handoff ordering publishes A first: A's obsolete-file cleanup preserves the locally modified file, but B then extracts with only B's previous_packages hashes (normally none for a newly claimed path), so write_verified renames B's file over the preserved configuration. Transfer the releasing owner's config hash to the claimant or otherwise preserve the live file across the handoff to avoid irreversible user configuration loss.
AGENTS.md reference: AGENTS.md:L76-L76
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 42ddce5 修复：journal 中已有的 previous_packages 现在先构造 physical path → config hash 映射；同事务接管该路径的新包会继承释放方哈希。若 live config 已被用户修改，接管包保留原文件并将新内容写为 .sage-new。现有 ownership handoff 回归已改为配置文件场景，同时保留循环 handoff 原子拒绝；全量测试和两条 CI 均通过。
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 42ddce5887
ℹ️ 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".
| let plan = | ||
| sage_sys::ReconcilePlan::compute(&config, &installed, &available.universe, no_prune)?; | ||
| let desired: Vec<_> = config.packages.iter().cloned().collect(); | ||
| apply_packages(root, &desired, Some("system"), false, false, dry_run).await?; |
There was a problem hiding this comment.
When switching from an installed provider to a conflicting replacement, this call discards plan.install and runs a second solve in apply_packages, where every installed package is made a root. For example, if installed glibc conflicts with preferred musl, ReconcilePlan::compute selects musl and schedules glibc for removal, but the second solve must retain glibc and backtracks to it, installs no musl, and then lines 1146–1152 remove glibc anyway, leaving the virtual dependency unsatisfied and persisting a binding to an uninstalled provider. Fresh evidence after the earlier provider-preflight fix is this divergence between the validated plan and the separate publication solve.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 15403b1 修复：rebuild 不再调用 apply_packages 二次求解，而是按依赖顺序直接发布已由 ReconcilePlan 验证的 plan.install 精确版本集合，然后执行同一 plan 的 remove 与 binding 替换。这样 Provider 切换不会在验证后分叉；现有 Provider 切换、冲突 binding 和完整 binding 回归均通过，两条 CI 通过。
| path.display() | ||
| ))); | ||
| } | ||
| file.set_permissions(std::fs::Permissions::from_mode(0o600))?; |
There was a problem hiding this comment.
When the lock namespace was writable before Sage starts, an attacker can pre-create operation.lock as a hard link to an attacker-owned file outside the namespace. O_NOFOLLOW and is_file() both accept that inode, and this line changes the external file's permissions while the attacker retains access through the other link and can later hold its flock to block all Sage operations. The directory-hardening fix does not inspect the file's owner or link count; reject foreign or multiply linked existing files before chmod and locking.
AGENTS.md reference: AGENTS.md:L77-L77
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 15403b1 修复：打开 operation.lock 后、chmod/flock 前检查 regular file、effective owner 及 nlink == 1；foreign 或 multiply linked inode 立即拒绝。现有 lock namespace 回归新增外部 hard-link fixture，并保留 symlink 与 permissive-directory 覆盖；全量测试和两条 CI 均通过。
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 15403b1cfd
ℹ️ 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".
| no_prune: bool, | ||
| ) -> Result<Self, SysError> { | ||
| let mut roots = config.package_keys("main/system")?; | ||
| let roots = config.package_keys("main/system")?; |
There was a problem hiding this comment.
When a [providers] entry is not also listed in packages and no desired package currently depends on its virtual symbol, removing the configured package from roots leaves no proxy selection for that interface. The plan then schedules an installed provider for pruning, omits its binding through the later filter_map, and can fail while rendering services after already removing the configured init implementation. Keep configured provider packages in the desired solution while deriving bindings from actual proxy selections where available.
AGENTS.md reference: AGENTS.md:L16-L20
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 22706cb 修复：先用 soft preference 求解真实 virtual proxy；仅对没有被任何 desired dependency 引用的配置接口，把其显式 Provider 加入 roots 后重算，并验证该包实际 provides 对应 symbol。这样 init 等独立配置 Provider 会保留，同时 libc 等活跃接口仍可冲突回溯。新增未引用 virtual/init 的 systemd 保留与 binding 回归；CI 通过。
| } | ||
| if !dry_run && !changes.is_empty() { | ||
| let database = sage_db::SageDatabase::open(&db_path)?; | ||
| publish_packages( |
There was a problem hiding this comment.
When rebuilding from an installed provider such as glibc to musl while an installed package depends on virtual/libc, this newly added publication installs musl first, but the subsequent remove_packages call still treats removed.provides.contains("virtual/libc") as proof that glibc is uniquely required and does not check the newly installed provider. The rebuild therefore fails after publishing musl but before removing glibc, persisting bindings, or rendering services. Fresh evidence after the exact-plan fix is that publication now succeeds before this unchanged dependency guard rejects the planned removal.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 22706cb 修复：remove dependency guard 在判断旧 Provider 必需前，会检查本次不删除的已安装包是否以相同 channel/slot/version constraint 提供同一 concrete/virtual dependency；已安装替代 Provider 存在时允许按 plan 删除旧 Provider。完整 Provider 切换与全量门禁通过。
| if normal_index == normal_components { | ||
| harden_lock_directory(&next, path)?; | ||
| } |
There was a problem hiding this comment.
When an ancestor of the private lock directory, such as a misconfigured writable /run, can be renamed by another user, hardening only the final /run/sage descriptor does not make the namespace replace-safe: the attacker can rename that directory after this check, create a replacement, and cause the next Sage process to lock a different inode. Fresh evidence after the directory-hardening change is that every intermediate directory is opened but none is checked for ownership or non-replaceability; reject unsafe ancestors or anchor the lock beneath a trusted root.
AGENTS.md reference: AGENTS.md:L77-L77
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 22706cb 修复：除最终私有目录外，openat walk 现在还在进入 sage 前验证直接祖先的 effective owner，且拒绝 group/other writable mode；不可信 /run 在创建私有 namespace 前失败。现有 lock 回归新增预建 0777 run/ fixture，并保留 symlink、hard-link 与 final-directory 修复覆盖；CI 通过。
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 22706cb9cb
ℹ️ 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".
| } | ||
| if !dry_run && !changes.is_empty() { | ||
| let database = sage_db::SageDatabase::open(&db_path)?; | ||
| publish_packages( |
There was a problem hiding this comment.
When sage rebuild --no-prune introduces a package that conflicts with an unconfigured installed package, ReconcilePlan::compute does not keep that installed package as a solver root and suppresses its removal because pruning is disabled. Publishing this plan therefore installs both packages successfully when their files do not overlap, leaving a system whose installed package set violates declared conflicts. Include every package retained by --no-prune in the validated solution before publication.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 4b5f4bc 修复：ReconcilePlan 在 no_prune 时把全部已安装 package key 加入 roots 后再求解，因此新 desired 包与任何保留包冲突都会在发布前无解。该场景并入现有 dependency-closed reconciliation 测试；全量门禁和两条 CI 通过。
| let lock_root = std::fs::canonicalize(&cli.root) | ||
| .with_context(|| format!("cannot resolve target root {}", cli.root.display()))?; | ||
| let lock_path = under_root(&lock_root, Path::new("/run/sage/operation.lock")); |
There was a problem hiding this comment.
When --root is a symlink whose directory can be modified concurrently, only the lock path uses its canonical destination; all subsequent operations still use cli.root. An attacker or orchestrator can retarget the symlink after lock acquisition, causing Sage to mutate a different sysroot while holding the first root's lock, so another process can mutate that second root concurrently. Use the resolved root for the entire operation, not just for constructing the lock path.
AGENTS.md reference: AGENTS.md:L77-L77
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 4b5f4bc 修复：execute 现在将 cli.root 本身替换为 canonicalized path，lock path 与后续 sync/install/remove/rebuild/query 全部使用同一个已解析 root，不再继续引用可被重定向的原始 symlink。全量门禁和两条 CI 通过。
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b5f4bc480
ℹ️ 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".
| &database, | ||
| &available, | ||
| &config.system.architecture, | ||
| &changes, |
There was a problem hiding this comment.
When reconciliation replaces an installed package with one that owns any of the same paths, only plan.install is passed into this publication transaction, so preflight treats the package in the later plan.remove set as a permanent owner and rejects the replacement. Fresh evidence after the earlier handoff fix is that rebuild now processes removals only after this call; provider or conflict replacements with overlapping files therefore still cannot converge. Publish installs and removals as one handoff-aware transaction, including configuration preservation and recovery state.
AGENTS.md reference: AGENTS.md:L76-L76
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 d9d2f07 修复：rebuild 将 plan.remove 快照与 plan.install 一起写入现有 Install journal。恢复先释放 retired package 的 DB ownership、发布 replacement，再清理无人持有路径；previous config hashes 与旧 trigger 文档同 journal 保留，支持配置 handoff、PostRemove/PostChange 和崩溃重放。重叠 ownership 不再被视为永久冲突；完整门禁和 CI 通过。
| [dependencies] | ||
| sage = { workspace = true } | ||
| anyhow = { workspace = true } | ||
| sage = { workspace = true, features = ["torture"] } |
There was a problem hiding this comment.
With the documented root-level cargo build --release command, sage-tests is one of the workspace's default members, so this dependency activates and unifies sage/torture while the production sage binary is built. Consequently the release binary includes the /run/sage/crash-point path that can deliberately abort an in-progress package operation. Activate this feature only in dedicated torture invocations or exclude the test crate from the default release workspace build.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 d9d2f07 修复：workspace default-members 仅包含生产 crates/sage，CI 改为显式 --workspace 测试/Clippy。已执行默认 cargo build --release 并扫描 target/release/sage，确认不含 crash-point、abort: 或 injected-crash 字符串；CI 通过。
| roots.sort(); | ||
| roots.dedup(); | ||
| if no_prune { | ||
| roots.extend(installed.iter().map(|package| package.key.clone())); |
There was a problem hiding this comment.
When an installed package or its exact version has disappeared from the synchronized repository, adding its key as a solver root does not actually retain that installed release: SageProvider contains only repository releases, and the lock is honored only when its version remains available. Thus rebuild --no-prune either upgrades an undeclared retained package to another repository version or fails with no solution when the key is gone, despite no pruning being requested. Add installed releases as exact solver candidates when validating the retained set.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 d9d2f07 修复：--no-prune 求解前复制候选 universe，并为仓库缺失的 installed key/version 注入由其持久化 dependencies/provides/conflicts 构造的精确 release；锁定版本因此可继续作为保留 root。现有 reconciliation 测试新增缺失 old release 的 no-prune 成功回归；CI 通过。
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9d2f07084
ℹ️ 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".
| let retired = installed | ||
| .iter() | ||
| .filter(|package| plan.remove.contains(&package.key)) | ||
| .cloned() |
There was a problem hiding this comment.
When an installed package outside main/system depends on a package in main/system—for example, a retained main/runtime package with main/system/libc or virtual/libc constraints—ReconcilePlan::compute does not include that retained package as a root during normal pruning, yet this new retirement path bypasses remove_packages' dependency guard and removes every key in plan.remove. A successful rebuild can therefore leave the retained package with an unsatisfied dependency; include all packages that will remain installed in the reconciliation solve, or validate their dependencies before retiring the plan.
AGENTS.md reference: AGENTS.md:L22-L23
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 1b05a59 修复：ReconcilePlan 现在把所有不会被本次 plan 删除的 cross-channel installed 包加入 roots；若其 exact key/version 已从仓库消失，也会由持久化 dependencies/provides/conflicts 注入临时候选。新增 main/runtime 包依赖 main/system/lib 且 runtime release 不在仓库的回归，确认 plan 保留 runtime 并安装 system 依赖；完整 workspace 测试、quick、固定 seed 随机序列和两条 CI 均通过。
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b05a594c7
ℹ️ 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".
| if !retained.is_empty() { | ||
| roots.extend(retained.iter().map(|package| package.key.clone())); |
There was a problem hiding this comment.
When --no-prune retains an installed package whose current version conflicts with a newly desired package, adding only its key as an unconstrained root allows the solver to select another repository version and include an unexpected upgrade in plan.install. Fresh evidence after the unavailable-release fix is that SageSolver::choose_version treats with_locked entries only as preferences and falls back to another matching version during backtracking; for example, retained old v1 conflicting with app will silently become v2 if v2 is available and compatible. Express retained packages as exact-version roots so --no-prune either preserves their installed releases or rejects the reconciliation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
To use Codex here, create an environment for this repo.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b05a594c7
ℹ️ 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".
| && dependency | ||
| .slot | ||
| .as_deref() | ||
| .is_none_or(|slot| slot == candidate.key.slot) |
There was a problem hiding this comment.
When sage remove removes lib:0 while lib:1 is also installed, this replacement check treats a dependency with no explicit slot as matching lib:1. The solver instead resolves an omitted slot to DEFAULT_SLOT, so a dependent declared simply as lib requires lib:0; accepting the nondefault package lets removal succeed and leaves the installed dependency unsatisfied. Treat None as slot 0 here rather than as a wildcard.
AGENTS.md reference: AGENTS.md:L22-L23
Useful? React with 👍 / 👎.
|
@codex address these feedback |
|
To use Codex here, create an environment for this repo. |
Convert the schema u32 mode through the host mode_t before calling nix so Darwin and Linux compile the same archive path. Verified with cargo test --all-targets on macOS. Co-authored-by: Codex <codex@openai.com>
Replace retry sleeps with the daemon readiness stream, keep stderr drained, and scope ELF assertions to Linux. Verified by the focused git fixture and the 63-test macOS baseline. Co-authored-by: Codex <codex@openai.com>
Keep configured providers as solver preferences, derive LMDB bindings from the actual solution, and stop rebuild from requesting preferences as hard roots. The regression proves conflict backtracking from musl to glibc. Co-authored-by: Codex <codex@openai.com>
Validate every payload header before the first write, reject duplicate canonical paths and unsafe symlink targets, and expose explicit rename/write fault boundaries for the Torture Lab. Co-authored-by: Codex <codex@openai.com>
Add all-or-nothing batch publication, pre-write/pre-commit fault injection, full ownership readback, and nonblocking host-lock probes for deterministic concurrency tests. Co-authored-by: Codex <codex@openai.com>
Preflight archive identity and ownership before journaling, checkpoint completed trigger groups, use collision-resistant operation IDs, add abrupt-failure hooks, and provide count/verify consistency commands. Co-authored-by: Codex <codex@openai.com>
Validate repository names, aliases, signing-key paths and target roots before they participate in cache or rootfs path construction. Regression fixtures cover each traversal source. Co-authored-by: Codex <codex@openai.com>
Add signed package fixtures, per-test rootfs and LMDB isolation, a reference-model random state machine, crash/retry matrices, path attacks, real process concurrency, state audits and repeatable benchmarks. Co-authored-by: Codex <codex@openai.com>
Run the quick hermetic gate in normal CI and add manual/nightly jobs for long fixed-seed state machines and 10,000-package release benchmarks. Co-authored-by: Codex <codex@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8779f61cba
ℹ️ 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".
| let class: InitRclass = toml::from_str(text)?; | ||
| validate_schema(class.schema_version)?; | ||
| Ok(class.service_generator) |
There was a problem hiding this comment.
When a provider archive contains syntactically valid TOML but an unusable renderer—for example target_path = "../../etc/${service.name}" while at least one service is installed—this function succeeds because it only deserializes and checks schema_version. preflight_packages therefore marks the renderer checked, publishes or retires packages, and then render_services fails at the durable rebuild-services stage, causing every later mutating command to retry the same failure. Fresh evidence beyond the prior malformed-renderer fix is that semantic path/template and supported_types checks still occur only during rendering; validate the renderer against the planned service set before creating the journal, or provide a rollback path.
AGENTS.md reference: AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2a112bc. preflight_packages now reconstructs the post-transaction service declaration set, selects the renderer from verified archive payloads or retained state, and runs the side-effect-free TemplateServiceGenerator::validate_service_set before creating the journal. This validates target/template expansion, sysroot containment, supported service types, and command-template paths, so unsafe targets such as ../../etc/${service.name} fail before package publication.
Added regression coverage for unsafe renderer paths and unsupported service types, including assertions that package, filesystem, configuration, provider, and journal state remain unchanged.
Nix verification:
cargo fmt --all -- --check: passedcargo clippy --workspace --all-targets -- -D warnings: passedcargo test -p sage-tests --test torture -- --nocapture: 17 passedcargo test --workspace --all-targets -- --skip journal_recovery_reconciles_an_init_slot_switch --skip elf_scanner_reads_dynamic_dependencies --skip private_runpaths_are_relative_and_passed_without_a_shell --skip git_fetch_materializes_recursive_network_submodules: 71 passed
The unfiltered workspace run reaches four pre-existing host-path-dependent cases that require tools under /bin or other host helper paths unavailable in this Nix shell; the changed torture suite and all remaining workspace tests pass.
Validate planned service generations before package publication so unsafe targets and unsupported service types cannot strand recovery journals. Co-authored-by: OpenAI Codex <codex@openai.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a112bc531
ℹ️ 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".
| package.provides.clone(), | ||
| ); | ||
| release.conflicts.clone_from(&package.conflicts); | ||
| universe.insert(release); |
There was a problem hiding this comment.
When an installed provider release missing from the repository is reconstructed here while another repository version of the same PackageKey no longer advertises that symbol, PackageUniverse indexes the key as a provider and SageProvider::build considers every version under that key without rechecking each release's provides. If a new root forces the newer version, an unconfigured virtual dependency can therefore be resolved to a release that does not actually provide it, and rebuild installs a dependency-broken state; proxy candidates must be filtered using the exact release metadata.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 eabb63b 修复。虚拟 Provider 索引仍保持按 PackageKey 的紧凑结构，但构造 proxy candidate 时现在会读取精确 (PackageKey, Version) release，并确认该 release 自身仍声明目标 symbol；同一 key 下已移除 provides 的新版不会再被旧版的索引项错误放行。
回归覆盖：已安装且仓库中缺失的 provider v1 提供 virtual/runtime-api，仓库 v2 已移除该声明，同时新增 root 强制 provider >= 2.0-1。修复前 solver 会错误接受 v2；修复后按预期返回无解，避免生成依赖损坏的 rebuild plan。
验证：cargo fmt --all -- --check、cargo clippy --workspace --all-targets -- -D warnings、71 项非主机路径依赖的 workspace tests 与全部 18 项 Torture tests 通过。完整未过滤运行仅命中既有 Nix 环境 /bin/rm 缺失用例。
| @@ -0,0 +1,767 @@ | |||
| //! Hermetic, model-checked fixtures for Sage package-manager torture testing. | |||
There was a problem hiding this comment.
The reviewed tree contains 15,896 lines across workspace *.rs files, and this commit adds 3,586 Rust lines compared with its parent, moving the project substantially farther beyond the repository's strict 9,000-line ceiling. Reduce or consolidate the added implementation and test infrastructure before merging.
AGENTS.md reference: AGENTS.md:L24-L25
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 75389e9 按项目决定更新文档：删除 Workspace Rust 的 9,000 行硬上限，不再以最大 SLOC 作为合并门槛；核心原则仍明确保留 KISS，要求优先采用直接、清晰、易维护的实现，避免不必要抽象与重复代码，并继续强调关键路径的精简和性能。
这是文档约束调整，没有引入替代的数值上限。
| for package in &installed { | ||
| for relative in &package.files { | ||
| let target = self.root().join(relative); |
There was a problem hiding this comment.
The new audit only queries owners for paths reached from installed packages, so a stale files LMDB row whose path is absent from every package record is never added to snapshot.owners and the audit still succeeds. That is precisely one direction of package/ownership corruption this torture gate claims to detect; iterate the complete ownership table and verify every entry points back to an installed package and listed file.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 22d3427 修复。SageDatabase::file_owners 以 LMDB key order 遍历完整 files reverse index；Torture snapshot 不再只从 installed package paths 做点查。audit 从 package records 构造完整期望 ownership map，对 owner 列表排序后与整张反向索引比较，因此缺失、额外、孤立或 owner 不匹配的 row 都会失败。
新增 audit_rejects_orphaned_reverse_ownership_rows：安装真实包后直接注入一个不属于任何 package record、且指向不存在 package key 的 stale LMDB row。修复前 audit 忽略该 row；修复后稳定拒绝。
验证：格式检查与 Clippy 零警告；71 项非主机路径依赖的 workspace tests 和全部 18 项 Torture tests 通过。
Require each virtual proxy candidate to advertise the requested symbol in its own release metadata. Cover reconstructed installed providers whose newer repository release drops that symbol. Co-authored-by: OpenAI Codex <codex@openai.com>
Expose an ordered snapshot of every reverse file-owner row and compare it with the installed package records. Add a torture regression for an orphaned ownership entry. Co-authored-by: OpenAI Codex <codex@openai.com>
Remove the numeric workspace line limit while retaining explicit guidance for direct, maintainable code and lean critical paths. Co-authored-by: OpenAI Codex <codex@openai.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75389e9ea7
ℹ️ 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".
| package.provides.clone(), | ||
| ); | ||
| release.conflicts.clone_from(&package.conflicts); | ||
| universe.insert(release); |
There was a problem hiding this comment.
When an installed provider v1 has disappeared from the repository, this reconstruction adds its symbol to the universe’s key-level provider index; if repository v2 of the same PackageKey no longer advertises that symbol, SageProvider::build still applies a conflicts = ["virtual/foo"] marker to every version under the indexed key without rechecking each release’s provides. A rebuild that should move the provider to v2 can therefore report no solution when another desired package conflicts with virtual/foo. Apply the same exact-release provider check used for proxy candidates when expanding virtual conflicts.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
To use Codex here, create an environment for this repo.
There was a problem hiding this comment.
已在 061029a 做最小修复：展开 virtual conflicts 时，先检查精确 (PackageKey, Version) release 自身是否仍声明该 symbol，再应用 conflict marker。保留现有 key-level 索引，不引入新抽象。
复用 reconciliation 测试覆盖：已安装且仓库中缺失的 provider v1 提供 virtual/runtime-api，仓库 v2 不再提供；同时声明 provider 和与该 symbol 冲突的 guard 时，现在可以升级到 v2。移除 v2 后，v1 的真实冲突仍返回无解。回归测试已确认修复前失败、修复后通过。
验证：cargo fmt --check、cargo clippy --workspace --all-targets -- -D warnings 通过。完整 cargo test --workspace 未全绿：当前 Nix 环境缺少 /bin/rm、/bin/ls，另有 git submodule fixture 返回 ENOENT。排除这 4 项环境失败后，其余 92 项测试（含全部 18 项 Torture tests）及 doc-tests 通过。
提交包含 Co-authored-by: OpenAI Codex <codex@openai.com>。未合并 PR。
Check each release provides the virtual symbol before adding conflict markers. Cover upgrades from reconstructed installed providers and retain rejection of actual provider conflicts. Co-authored-by: OpenAI Codex <codex@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 061029ac6d
ℹ️ 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".
| if let Some(owner) = planned.insert(path.clone(), key.clone()) { | ||
| if owner != *key { | ||
| bail!("transaction packages {owner} and {key} both own {path}"); |
There was a problem hiding this comment.
Reject the duplicate even when owner == *key. If an archive payload already contains a generated declaration destination—for example usr/share/sage/services/foo.toml alongside a .METADATA/service.toml entry named foo—preflight accepts the path twice, extraction installs the indexed payload, and declaration publication immediately overwrites it with different contents. The installed package record also retains the duplicate path even though the reverse index has only one owner, so the installed filesystem and ownership metadata no longer faithfully represent the verified payload.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 5d962c2 最小修复：移除 owner != *key 豁免，同一包的 payload 与生成声明占用相同路径时也在 preflight 拒绝，不增加新抽象。
新增回归构造同时包含 usr/share/sage/services/foo.toml payload 和 .METADATA/service.toml 中 foo 声明的签名包。旧代码错误安装；修复后返回 ownership collision，且目标文件、package records、reverse ownership rows 和 recovery journal 均未写入。
验证：cargo fmt --check、cargo clippy --workspace --all-targets -- -D warnings 通过。完整 workspace test 仍被当前 Nix 环境缺少 /bin/rm 的既有用例阻断；排除前轮已确认的 4 项环境失败后，93 项测试（含全部 19 项 Torture tests）及 doc-tests 通过。
提交已包含 Co-authored-by: OpenAI Codex <codex@openai.com>，未合并 PR。
Reject duplicate planned paths even when both claims belong to the same package. Add a regression for a payload colliding with a generated service declaration before publication or journaling. Co-authored-by: OpenAI Codex <codex@openai.com>
There was a problem hiding this comment.
💡 Codex Review
sage/crates/sage/src/package_ops.rs
Lines 1004 to 1005 in 5d962c2
Include obsolete upgrade files in removal trigger paths
When a same-key upgrade drops a previously owned path, this branch removes the file but records it only in modified; removed_paths is populated exclusively for fully retired packages below. Consequently an old trigger configured only for post-remove and matching that obsolete path is present in removal_trigger_documents but receives an empty path set and never runs, while the later post-change pass excludes it by event. Record these obsolete paths as removals as well.
AGENTS.md reference: AGENTS.md:L18-L18
ℹ️ 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".
| target_path(sysroot, Path::new(program)).map_err(|error| { | ||
| SysError::Invalid(format!("invalid {kind} command program: {error}")) | ||
| })?; |
There was a problem hiding this comment.
When a selected provider's syntactically valid rclass declares a missing validate_command program, or a missing enable_command for an enabled service, this validation only normalizes the path and accepts it. Package publication then completes before run_validation rejects the absent executable at rebuild-services, leaving a journal that every later mutating command retries. Fresh evidence after the prior semantic-validation fix is that executable availability is still never checked against retained and planned package paths.
AGENTS.md reference: AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
| if provider_changed || removed || disabled { | ||
| previous_generator.disable_service(service, root)?; | ||
| previous.generator.disable_service(service, root)?; |
There was a problem hiding this comment.
When switching init providers and the retired provider owns the executable named by its persisted disable_command—for example Loom owning /usr/bin/loomctl—the package stage deletes that executable before this call runs. disable_service then fails its executable check at rebuild-services, so the switch and every subsequent journal-recovery attempt remain stuck. Fresh evidence after persisting the old generator is that its command binaries are not persisted or retained; perform old-provider cleanup before retirement or otherwise keep those programs available.
AGENTS.md reference: AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
Record dropped same-key upgrade paths in the journal removal set. Cover replay of an old post-remove-only trigger after a trigger-stage interruption, using executable package fixtures. Co-authored-by: OpenAI Codex <codex@openai.com>
Validate provider command executables against the final package filesystem, including executable modes, symlink targets, and payload service declarations. Reject missing enabled declarations before journaling. Run previous-generation cleanup in a durable stage before package publication removes its programs or runtime dependencies. Persist the cleared generation before advancing so recovery does not call retired programs. Cover missing command variants, fresh executable payloads, non-executable replacements, removed targets, provider switching, and interruptions across cleanup and package publication. Co-authored-by: OpenAI Codex <codex@openai.com>
|
本轮将三条新反馈及相邻的事务边界一起修复，避免只补单个报错点。
验证：三组回归在原代码上失败，修复后通过；本地 96 项非环境依赖测试、全部 21 项 Torture tests（包含在 96 项中）、doc-tests、格式检查和 Clippy 通过。固定 seed 最终提交 两个提交均包含 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ce73b6c53
ℹ️ 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".
| for service in &previous.services { | ||
| previous.generator.disable_service(service, root)?; | ||
| previous.generator.remove_service(service, root)?; |
There was a problem hiding this comment.
When the rendered state contains an installed service absent from previous.enabled, any rebuild with package changes still invokes the provider's disable command for that never-enabled service. If an init provider rejects disabling an inactive or nonexistent unit, the journal remains at rebuild-cleanup, and every later mutating command retries the same failure. Gate disable_service on membership in previous.enabled while still removing all stale native definitions.
AGENTS.md reference: AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
Co-authored-by: Codex <codex@openai.com>
Restore provider resolution, declarative reconciliation, and init rendering to the main-branch implementation. Remove their rebuild continuation, renderer/program preflight, cleanup stages, auxiliary APIs, and dedicated regressions from this PR. Retain the hermetic package lab, package recovery and ownership gates, and their directly required fixes; keep the full ownership audit behind the torture feature. Co-authored-by: Codex <codex@openai.com>
|
@codex review Please review the current diff at 2a62235 against main within the PR's original package-manager Torture Lab scope. It retains hermetic install/upgrade/remove tests and their directly exercised archive, ownership, journal, and lock fixes. Provider resolution, declarative rebuild policy, and init renderer lifecycle extensions have been removed and restored to the base implementation; do not treat unchanged base behavior as a regression introduced by this diff. The PR description contains the current scope and validation evidence. No merge is requested. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a62235f67
ℹ️ 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".
| sage_sys::TriggerEngine::execute_triggers_for( | ||
| &removal_triggers, | ||
| &removed_paths, | ||
| root, | ||
| sage_sys::TriggerEvent::PostRemove, |
There was a problem hiding this comment.
When an upgrade removes an obsolete path and also removes the executable referenced by an old post-remove trigger—for example, the old package owns both a cache file and /usr/bin/update-cache, while v2 drops both—the package stage deletes the executable before this newly added call. TriggerEngine then rejects the missing binary when ignore_missing_binary is false, leaving the journal at triggers; every subsequent mutating command retries the same missing executable and cannot recover. Run old removal triggers while their command providers remain available, or reject this transaction during preflight.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 381475c 按原始 Torture Lab 范围撤回这项扩展：删除 upgrade 专用旧 PostRemove 调用、Install journal 的 removed_paths/removal_trigger_documents 字段及专用测试。普通 remove 的 PostRemove 和 upgrade 原有 PostChange 语义保持 main 行为；不再扩大到旧触发器工具生命周期管理。
Removed in 381475c as part of the requested scope reduction. The newly introduced upgrade-only old PostRemove behavior and its journal/test machinery are no longer in the PR; existing ordinary remove/upgrade trigger behavior remains.
| index: &[FileRecord], | ||
| previous_hashes: &BTreeMap<String, String>, | ||
| ) -> Result<ExtractionReport, ArchiveError> { | ||
| validate_package_payload(package, index)?; |
There was a problem hiding this comment.
For every package-manager install, preflight_packages already fully decompresses and hashes the archive via validate_package_payload; this unconditional call repeats that scan, after which extraction opens and decompresses the archive a third time to publish it. Large packages therefore incur three complete payload passes instead of the previous single pass, making decompression and hashing unnecessarily dominate the installation critical path. Use a prevalidated extraction path for package-manager callers while retaining fail-closed validation for direct callers.
AGENTS.md reference: AGENTS.md:L24-L26
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
已在 31c1795 修复：直接复用原有 extraction 实现，在本次 publish 已完成 preflight 时跳过第二次完整 payload 验证；每文件 hash 和 dirfd 安全写入仍保留。直接解包/构建依赖解包仍走 checked API，settle_journals 恢复入口也明确走 checked API，重新验证当前归档。普通安装从三遍完整 payload 遍历减少为预检+解包两遍，没有新增 journal schema 或用户配置。
最终验证通过：fmt、Clippy -D warnings、workspace all-targets（67 主测试 + 18 Torture）、quick 17步骤、seed 1517166630 的1000操作/1631步骤、release/no-default-features，生产二进制无fault markers。
Fixed in 31c1795 by reusing the existing extractor after same-run package preflight. Direct callers and journal recovery retain full validation; per-file integrity and safe writes remain enforced. All scoped gates pass.
Keep upgrade trigger behavior aligned with the main branch instead of extending the torture lab into old trigger-provider lifetime management. Remove the added install journal fields, specialized regression, and unused executable fixture setting. Co-authored-by: Codex <codex@openai.com>
Reuse the existing extraction implementation after package-manager preflight instead of rescanning the full payload. Direct extraction and journal recovery still validate their current archive; per-file integrity and dirfd-relative writes remain enforced. Co-authored-by: Codex <codex@openai.com>
|
@codex review Please review the final diff at 31c1795 against main within the original package-manager Torture Lab scope. The upgrade-only old PostRemove extension has also been removed, and same-run publication now reuses payload preflight while direct extraction and journal recovery remain checked. Provider/rebuild/init lifecycle extensions are restored to base, not part of this PR. The updated description lists the exact scope and passing checks; no merge is requested. |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Deferred work removed during scope reduction is now tracked separately:
Each issue contains the complete English text followed by the complete Chinese text, with original code snapshots, removal commits, review evidence, and bounded acceptance criteria. PR #35 was merged concurrently by 范围收敛时撤出的工作已单独追踪：
每项 issue 均为完整英文后附完整中文，包含原始代码快照、撤回提交、review 证据和明确的验收边界。在计划执行 squash 前，PR #35 已由 |
2 participants
Summary
Add a hermetic package-manager Torture Lab that drives real
sage::executeinstall, upgrade, and remove calls against an isolated rootfs, LMDB, signed package pool, cache, and channels.建立隔离的包管理器测试环境，验证真实包操作后的文件、数据库、完整 ownership 索引和恢复状态。
Scope
torture. Default release builds contain production Sage only.3ce73b6; this PR does not claim those findings are fixed.本次收敛净删 1,691 行，PR 差异从 28 个文件、
+4,514 / -268收敛到 23 个文件、+2,718 / -163。保留现有 KISS/当前需求优先的仓库指令；不修改 recipes。Validation
cargo fmt --all -- --checkcargo clippy --workspace --all-targets -- -D warningscargo test --workspace --all-targets: 67 integration tests and 18 Torture Lab tests pass.1517166630, 1,000 operations, 1,631 recorded steps pass.cargo build --release: passes; the resulting binary contains neither/run/sage/crash-pointnorinjected crash aftermarkers.cargo check -p sage --no-default-features: passes.These checks validate the scoped lab and package-operation gates, not end-to-end init-provider lifecycle correctness.