build: stabilize framework phase zero - #17
Conversation
Pin framework consumers to the immutable BumpyClock GPUI fork and add compatibility, publication, testing, and release gates while crates.io publication remains deferred.
WalkthroughThe PR replaces GPUI submodule guidance with immutable dependency pins and compatibility metadata, adds an ChangesGPUI release and validation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 24
🤖 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 @.agents/skills/update-gpui/SKILL.md:
- Line 7: Add a top-level “Update GPUI” heading immediately after the front
matter in SKILL.md, before the existing “Instructions” subsection, while
retaining “Instructions” as a level-two heading.
- Around line 41-48: Update the “Build, test, and check release plan” workflow
in SKILL.md to include the repository’s canonical formatting, spell-checking,
and dependency-check commands alongside the existing cargo test, Clippy,
publish-plan, and release-check gates. Use the exact project-standard commands
and preserve all existing validation steps.
In @.claude/skills/update-gpui/SKILL.md:
- Around line 41-48: Expand the “Build, test, and check release plan” workflow
to include explicit formatting, spell checking, dependency checks, and locked
metadata/workspace validation, or document the exact delegated command for each
gate. Keep the existing test, Clippy, publish-plan, and release-check commands,
and require all validation gates to pass before reporting the update as fully
validated.
In @.github/ISSUE_TEMPLATE/01_bug.md:
- Line 14: Update the GPUI field example in the issue template to explicitly
require both the exact version and full commit SHA, using comma-separated
wording rather than a slash that implies they are interchangeable.
In @.github/workflows/ci.yml:
- Around line 15-16: Add a top-level permissions block in the CI workflow
granting GITHUB_TOKEN only contents: read by default. Review the compile job and
any other jobs, adding narrower job-level permissions only when their steps
explicitly require them, without broadening the workflow-wide default.
- Around line 70-71: Replace every tag-based uses entry for actions/checkout and
actions-rust-lang/setup-rust-toolchain in the workflow, including all
occurrences in the referenced steps, with immutable full commit SHAs; preserve
or add version comments for readability without changing the workflow behavior.
- Line 70: Update every actions/checkout@v4 step in the validation jobs to set
persist-credentials to false. Preserve the existing checkout configuration and
ensure no validation checkout retains the default GITHUB_TOKEN.
- Line 71: Update the setup-rust-toolchain steps in the CI workflow to prevent
cache writes for pull request jobs by disabling caching or gating cache-save-if
to trusted branches; apply the same protection to both referenced setup steps
while preserving Rust toolchain setup.
In @.github/workflows/release.yml:
- Around line 1-8: Add a ref-keyed concurrency configuration to the “Release
Gate” workflow so runs for the same tag cannot overlap, while preserving the
existing push tag trigger and permissions.
- Around line 14-15: Update the actions/checkout@v4 step in the release workflow
to disable credential persistence by setting persist-credentials to false; leave
the subsequent setup-rust-toolchain step unchanged.
In `@docs/learned/app-platform-plan.md`:
- Line 112: Update the sibling references to gpui_linux/src/platform.rs:685 and
x11/window.rs:1619 in the same sentence to GitHub permalinks pinned to commit
d1a6019136eb9fb9386eaa047a7445c1032d8668, matching the existing gpui_windows
reference format.
In `@docs/learned/gpui-submodule.md`:
- Around line 2-10: Rename the documentation file to reflect its local GPUI
override workflow rather than a submodule, and update all references in
AGENTS.md, RELEASING.md, and LEARNINGS.md to use the new filename. Preserve the
document content and existing link targets aside from the path change.
- Around line 12-21: Update the GPUI coordinated-development instructions to
place the Cargo patch override in a non-tracked configuration location,
preferably $CARGO_HOME/config.toml or a local include ignored by the committed
configuration, instead of .cargo/config.toml. Preserve the existing patch crate
names and paths.
In `@LEARNINGS.md`:
- Around line 87-88: Update the wording in the dependency note by changing
“failed due GPUI workspace dependency inheritance” to “failed due to GPUI
workspace dependency inheritance,” without altering the surrounding guidance.
In `@xtask/src/main.rs`:
- Around line 589-600: Replace substring-based GPUI repository checks with
normalized equality using normalized_git_repository in validate_lockfile
(xtask/src/main.rs:589-600) and add_resolved_engine_nodes
(xtask/src/main.rs:1229-1232). Keep the revision comparison as a separate
explicit check, without using another contains test.
- Around line 831-854: Update validate_root_patches so detected GPUI workspace
root patches are appended to report.warnings rather than report.errors, matching
the soft-blocker behavior used by build_plan for coordinated local development.
Preserve the existing manifest and metadata error handling, and leave release
enforcement to release-check or --require-registry.
- Around line 1810-1829: Bound the network wait in registry_probe by applying an
explicit timeout to the cargo registry query, while preserving the existing
published, unpublished, and unknown result mapping. Ensure a slow or unreachable
crates.io request returns "unknown" promptly and remains fail-closed for
require_registry_gate; avoid adding retries.
- Around line 1594-1618: Update the registry-failure classification in the
package command flow around engine_registry_blocked and registry_failure so
expected unavailable-registry prerequisites are recognized without depending
solely on exact English cargo stderr phrases. Use a broader, stable signal
available from the command result or explicitly document and pin the
cargo/toolchain locale expectation; preserve the existing blocker message and
hard bail behavior for unrelated failures.
- Line 925: Replace the single-character string append in the output-building
code with the character-oriented append API, preserving the existing newline
output and ensuring Clippy passes with warnings denied.
- Around line 247-249: Prevent duplicate root-manifest validation between
validate_root_manifest and validate_all_manifests. Prefer excluding the root
Cargo.toml from the traversal in validate_all_manifests while preserving its
validation through validate_root_manifest, so workspace dependency errors are
reported once.
- Around line 1778-1808: Consolidate cargo_metadata and cargo_metadata_full by
introducing a shared helper that accepts a no_deps boolean, or make both
functions thin wrappers over one. Keep the shared command execution, error
handling, and JSON parsing in one place, adding --no-deps only when requested so
both variants remain synchronized.
- Around line 1514-1535: Update release_check so publish_plan is invoked only
once, passing options.require_registry to gate registry validation. Remove the
earlier conditional publish_plan call and its duplicate stage output, then keep
the remaining phase labels sequential and accurate for the single
publication-plan phase.
- Around line 1568-1585: Update both cargo package invocations in the
publishable package loop to apply --allow-dirty only when !require_registry;
keep the registry path strict so cargo package validates a clean working tree,
while preserving the existing dev convenience and validate_gpui_checkout
behavior.
In `@xtask/src/tests.rs`:
- Around line 551-571: The archive fixture and related package-inspection
helpers currently depend on host-specific tar commands; replace the tar
shell-outs in archive_fixture, inspect_package_files, and
inspect_normalized_manifest with a Rust tar/flate2 implementation that creates
and reads gzip-compressed archives using platform-independent path handling.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: b1e0a24a-b36f-45c6-be3f-14c4303a8ff7
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockexamples/system_monitor/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (42)
.agents/skills/update-gpui/SKILL.md.cargo/config.toml.claude/skills/update-gpui/SKILL.md.github/ISSUE_TEMPLATE/01_bug.md.github/workflows/ci.yml.github/workflows/release-docs.yml.github/workflows/release.yml.gitmodulesAGENTS.mdCargo.tomlLEARNINGS.mdLICENSE-AUDIT.mdREADME.mdRELEASING.mdTESTING.mdcompatibility.tomlcrates/app-manifest/Cargo.tomlcrates/app-manifest/LICENSE-APACHEcrates/app-manifest/README.mdcrates/app-storage/Cargo.tomlcrates/app-storage/LICENSE-APACHEcrates/app-storage/README.mdcrates/app/Cargo.tomlcrates/assets/Cargo.tomlcrates/assets/LICENSE-APACHEcrates/macros/Cargo.tomlcrates/macros/README.mdcrates/ui/Cargo.tomlcrates/webview/Cargo.tomlcrates/webview/LICENSE-APACHEdocs/.vitepress/theme/index.tsdocs/COMPATIBILITY.mddocs/docs/getting-started.mddocs/docs/index.mddocs/docs/installation.mddocs/learned/app-platform-plan.mddocs/learned/gpui-submodule.mdtypos.tomlvendor/gpuixtask/Cargo.tomlxtask/src/main.rsxtask/src/tests.rs
💤 Files with no reviewable changes (3)
- vendor/gpui
- .gitmodules
- typos.toml
| user_invocable: true | ||
| --- | ||
|
|
||
| ## Instructions |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a top-level heading after the front matter.
markdownlint flags ## Instructions as the first heading. Add a # Update GPUI heading before it and retain ## Instructions as a subsection.
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 7-7: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 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 @.agents/skills/update-gpui/SKILL.md at line 7, Add a top-level “Update GPUI”
heading immediately after the front matter in SKILL.md, before the existing
“Instructions” subsection, while retaining “Instructions” as a level-two
heading.
Source: Linters/SAST tools
| 5. Build, test, and check release plan: | ||
|
|
||
| 7. **Run clippy**: | ||
| ```bash | ||
| cargo clippy -- --deny warnings | ||
| cargo test --workspace --all-targets --locked | ||
| cargo clippy --workspace --all-targets --locked -- -D warnings | ||
| cargo xtask publish-plan | ||
| cargo xtask release-check | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Include all required validation gates in the skill.
The workflow covers tests, Clippy, publish-plan, and release-check, but omits formatting, spell checking, and dependency checks required by the repository instructions. Add the project’s canonical commands so following this skill cannot skip those gates.
🤖 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 @.agents/skills/update-gpui/SKILL.md around lines 41 - 48, Update the “Build,
test, and check release plan” workflow in SKILL.md to include the repository’s
canonical formatting, spell-checking, and dependency-check commands alongside
the existing cargo test, Clippy, publish-plan, and release-check gates. Use the
exact project-standard commands and preserve all existing validation steps.
Source: Coding guidelines
| 5. Build, test, and check release plan: | ||
|
|
||
| 7. **Run clippy**: | ||
| ```bash | ||
| cargo clippy -- --deny warnings | ||
| cargo test --workspace --all-targets --locked | ||
| cargo clippy --workspace --all-targets --locked -- -D warnings | ||
| cargo xtask publish-plan | ||
| cargo xtask release-check | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Include all required validation gates in this workflow.
The procedure runs tests, Clippy, compatibility, and release commands, but does not explicitly run formatting, spell checking, dependency checks, or locked metadata/workspace validation. Add those checks, or document the exact command that delegates each one, before reporting the update as fully validated.
🤖 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 @.claude/skills/update-gpui/SKILL.md around lines 41 - 48, Expand the “Build,
test, and check release plan” workflow to include explicit formatting, spell
checking, dependency checks, and locked metadata/workspace validation, or
document the exact delegated command for each gate. Keep the existing test,
Clippy, publish-plan, and release-check commands, and require all validation
gates to pass before reporting the update as fully validated.
Source: Coding guidelines
| fn cargo_metadata(root: &Path) -> Result<CargoMetadata> { | ||
| let output = Command::new("cargo") | ||
| .args(["metadata", "--locked", "--no-deps", "--format-version", "1"]) | ||
| .current_dir(root) | ||
| .output() | ||
| .with_context(|| format!("failed to run cargo metadata in {}", root.display()))?; | ||
| if !output.status.success() { | ||
| bail!( | ||
| "cargo metadata failed in {}:\n{}", | ||
| root.display(), | ||
| String::from_utf8_lossy(&output.stderr) | ||
| ); | ||
| } | ||
| serde_json::from_slice(&output.stdout).context("cargo metadata returned invalid JSON") | ||
| } | ||
|
|
||
| fn cargo_metadata_full(root: &Path) -> Result<CargoMetadata> { | ||
| let output = Command::new("cargo") | ||
| .args(["metadata", "--locked", "--format-version", "1"]) | ||
| .current_dir(root) | ||
| .output() | ||
| .with_context(|| format!("failed to run cargo metadata in {}", root.display()))?; | ||
| if !output.status.success() { | ||
| bail!( | ||
| "cargo metadata failed in {}:\n{}", | ||
| root.display(), | ||
| String::from_utf8_lossy(&output.stderr) | ||
| ); | ||
| } | ||
| serde_json::from_slice(&output.stdout).context("cargo metadata returned invalid JSON") | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
cargo_metadata and cargo_metadata_full are byte-identical apart from --no-deps.
Collapse into one function taking a no_deps: bool (or two thin wrappers over a shared helper) so the error contexts and argument list stay in sync.
♻️ Sketch
+fn run_cargo_metadata(root: &Path, extra: &[&str]) -> Result<CargoMetadata> {
+ let mut command = Command::new("cargo");
+ command
+ .args(["metadata", "--locked", "--format-version", "1"])
+ .args(extra)
+ .current_dir(root);
+ let output = command
+ .output()
+ .with_context(|| format!("failed to run cargo metadata in {}", root.display()))?;
+ if !output.status.success() {
+ bail!(
+ "cargo metadata failed in {}:\n{}",
+ root.display(),
+ String::from_utf8_lossy(&output.stderr)
+ );
+ }
+ serde_json::from_slice(&output.stdout).context("cargo metadata returned invalid JSON")
+}
+
+fn cargo_metadata(root: &Path) -> Result<CargoMetadata> {
+ run_cargo_metadata(root, &["--no-deps"])
+}
+
+fn cargo_metadata_full(root: &Path) -> Result<CargoMetadata> {
+ run_cargo_metadata(root, &[])
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn cargo_metadata(root: &Path) -> Result<CargoMetadata> { | |
| let output = Command::new("cargo") | |
| .args(["metadata", "--locked", "--no-deps", "--format-version", "1"]) | |
| .current_dir(root) | |
| .output() | |
| .with_context(|| format!("failed to run cargo metadata in {}", root.display()))?; | |
| if !output.status.success() { | |
| bail!( | |
| "cargo metadata failed in {}:\n{}", | |
| root.display(), | |
| String::from_utf8_lossy(&output.stderr) | |
| ); | |
| } | |
| serde_json::from_slice(&output.stdout).context("cargo metadata returned invalid JSON") | |
| } | |
| fn cargo_metadata_full(root: &Path) -> Result<CargoMetadata> { | |
| let output = Command::new("cargo") | |
| .args(["metadata", "--locked", "--format-version", "1"]) | |
| .current_dir(root) | |
| .output() | |
| .with_context(|| format!("failed to run cargo metadata in {}", root.display()))?; | |
| if !output.status.success() { | |
| bail!( | |
| "cargo metadata failed in {}:\n{}", | |
| root.display(), | |
| String::from_utf8_lossy(&output.stderr) | |
| ); | |
| } | |
| serde_json::from_slice(&output.stdout).context("cargo metadata returned invalid JSON") | |
| } | |
| fn run_cargo_metadata(root: &Path, extra: &[&str]) -> Result<CargoMetadata> { | |
| let mut command = Command::new("cargo"); | |
| command | |
| .args(["metadata", "--locked", "--format-version", "1"]) | |
| .args(extra) | |
| .current_dir(root); | |
| let output = command | |
| .output() | |
| .with_context(|| format!("failed to run cargo metadata in {}", root.display()))?; | |
| if !output.status.success() { | |
| bail!( | |
| "cargo metadata failed in {}:\n{}", | |
| root.display(), | |
| String::from_utf8_lossy(&output.stderr) | |
| ); | |
| } | |
| serde_json::from_slice(&output.stdout).context("cargo metadata returned invalid JSON") | |
| } | |
| fn cargo_metadata(root: &Path) -> Result<CargoMetadata> { | |
| run_cargo_metadata(root, &["--no-deps"]) | |
| } | |
| fn cargo_metadata_full(root: &Path) -> Result<CargoMetadata> { | |
| run_cargo_metadata(root, &[]) | |
| } |
🤖 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 `@xtask/src/main.rs` around lines 1778 - 1808, Consolidate cargo_metadata and
cargo_metadata_full by introducing a shared helper that accepts a no_deps
boolean, or make both functions thin wrappers over one. Keep the shared command
execution, error handling, and JSON parsing in one place, adding --no-deps only
when requested so both variants remain synchronized.
| fn archive_fixture(manifest: &str, include_license: bool) -> (TempDir, CargoPackage) { | ||
| let directory = TempDir::new().unwrap(); | ||
| let root = directory.path(); | ||
| let package_root = root.join("staging/fixture-1.0.0"); | ||
| fs::create_dir_all(root.join("target/package")).unwrap(); | ||
| fs::create_dir_all(&package_root).unwrap(); | ||
| fs::write(package_root.join("Cargo.toml"), manifest).unwrap(); | ||
| fs::write(package_root.join("README.md"), "# fixture").unwrap(); | ||
| if include_license { | ||
| fs::write(package_root.join("LICENSE-APACHE"), "fixture").unwrap(); | ||
| } | ||
| let status = Command::new("tar") | ||
| .args([ | ||
| "-czf", | ||
| "../target/package/fixture-1.0.0.crate", | ||
| "fixture-1.0.0", | ||
| ]) | ||
| .current_dir(root.join("staging")) | ||
| .status() | ||
| .unwrap(); | ||
| assert!(status.success()); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
tar shelling in the archive fixture is host-dependent.
Command::new("tar") with -czf and a relative ../target/package/... destination assumes a GNU/bsdtar on PATH with a POSIX-ish path handling. Per TESTING.md, unit tests only run on Ubuntu today, so this is latent — but if the test lane ever extends to Windows, these fixtures (and inspect_package_files/inspect_normalized_manifest, which also shell out to tar) will need a Rust tar/flate2 implementation instead.
🤖 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 `@xtask/src/tests.rs` around lines 551 - 571, The archive fixture and related
package-inspection helpers currently depend on host-specific tar commands;
replace the tar shell-outs in archive_fixture, inspect_package_files, and
inspect_normalized_manifest with a Rust tar/flate2 implementation that creates
and reads gzip-compressed archives using platform-independent path handling.
Require least-privilege immutable Actions, exact GPUI source matching, bounded registry probes, and clean strict packaging so deferred registry blockers cannot mask unrelated failures.
Keep the registry-classifier regressions clean under the repository's pinned Rust 1.95 Clippy gate.
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 `@xtask/src/main.rs`:
- Around line 1862-1889: Update registry_probe to use the process status
returned by wait_timeout when it returns Some(status), avoiding a second wait
via wait_with_output. Read the existing stderr pipe separately, then pass
status.success() and the collected stderr to registry_probe_status; preserve the
timeout and error paths returning "unknown".
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 376331e7-8578-4fe6-92c4-684b345d4274
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
.agents/skills/update-gpui/SKILL.md.claude/skills/update-gpui/SKILL.md.github/ISSUE_TEMPLATE/01_bug.md.github/workflows/ci.yml.github/workflows/release-docs.yml.github/workflows/release.yml.github/workflows/test-docs.ymlCargo.tomlLEARNINGS.mddocs/learned/app-platform-plan.mddocs/learned/gpui-submodule.mdxtask/Cargo.tomlxtask/src/main.rsxtask/src/tests.rs
| fn registry_probe(package: &str, version: &str) -> String { | ||
| let mut child = match Command::new("cargo") | ||
| .args([ | ||
| "info", | ||
| &format!("{package}@{version}"), | ||
| "--registry", | ||
| "crates-io", | ||
| ]) | ||
| .stdout(Stdio::null()) | ||
| .stderr(Stdio::piped()) | ||
| .spawn() | ||
| { | ||
| Ok(child) => child, | ||
| Err(_) => return "unknown".into(), | ||
| }; | ||
| let output = match child.wait_timeout(REGISTRY_PROBE_TIMEOUT) { | ||
| Ok(Some(_)) => child.wait_with_output().ok(), | ||
| Ok(None) | Err(_) => { | ||
| let _ = child.kill(); | ||
| let _ = child.wait(); | ||
| return "unknown".into(); | ||
| } | ||
| }; | ||
| match output { | ||
| Some(output) => registry_probe_status(output.status.success(), &output.stderr).into(), | ||
| None => "unknown".into(), | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does calling Child::wait() a second time after wait-timeout crate's wait_timeout returns Some(status) on Unix risk an ECHILD error, or does the crate keep std's Child state consistent for a safe re-wait?
💡 Result:
Calling std::process::Child::wait after wait_timeout has already successfully reaped the child process on Unix does not inherently trigger an ECHILD error, but it is dangerous and generally unsafe [1][2]. The wait-timeout crate does not keep the standard library's Child state synchronized with the actual process status [3][4]. When wait_timeout returns Some(status), it has already successfully reaped the child process using waitpid [1][2]. However, the std::process::Child instance remains unaware that the process has been reaped [3][4]. Because wait internally calls waitpid on the child's PID, and that process ID no longer corresponds to a valid child process (or has already been collected), calling wait again is effectively a use-after-free or a race condition [5][3][4]. On many systems, a second call to wait on a process that has already been collected may fail with ECHILD, or worse, if the original PID has been recycled by the operating system for a different, unrelated process, wait might incorrectly block on or reap that unrelated process [5][3][4]. To safely handle this, the crate's documentation and common wrappers (such as those found in the rusty-fork crate) advise against calling wait again if wait_timeout has returned Some(status) [6][3][4]. You should treat the status returned by wait_timeout as the final, definitive exit status of that child [6][7]. If you need to manage this state, you must manually track whether the child has already been reaped and avoid further calls to standard wait methods [3][4].
Citations:
- 1: https://doc.cuprate.org/src/wait_timeout/unix.rs.html
- 2: https://cel.cs.brown.edu/aquascope/doc/src/wait_timeout/unix.rs.html
- 3: https://doc.cuprate.org/src/rusty_fork/child_wrapper.rs.html
- 4: https://docs.rs/rusty-forkfork/latest/rusty_forkfork/struct.ChildWrapper.html
- 5: https://man7.org/linux/man-pages/man2/wait.2.html
- 6: https://docs.rs/wait-timeout/latest/wait_timeout/index.html
- 7: https://alexcrichton.github.io/wait-timeout/wait_timeout/trait.ChildExt.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant function and nearby tests.
sed -n '1840,1915p' xtask/src/main.rs
printf '\n--- tests mentioning registry_probe ---\n'
rg -n "registry_probe|wait_timeout|registry_probe_status_is_fail_closed_for_unknown_failures" xtask/src/main.rs
printf '\n--- wait_timeout usage in this file ---\n'
rg -n "wait_timeout|wait_with_output|wait\(" xtask/src/main.rsRepository: BumpyClock/gpui-component
Length of output: 3236
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the dependency version and whether the crate source is available locally.
rg -n '^wait-timeout|wait_timeout' Cargo.lock Cargo.toml xtask/Cargo.toml || true
printf '\n--- cargo registry source candidates ---\n'
python3 - <<'PY'
import os
from pathlib import Path
home = Path.home()
cands = [
home / ".cargo" / "registry" / "src",
home / ".cargo" / "git" / "checkouts",
]
for c in cands:
print(c, "exists" if c.exists() else "missing")
if c.exists():
for p in list(c.iterdir())[:10]:
print(" ", p)
PY
printf '\n--- search local cargo registry for wait_timeout source if present ---\n'
python3 - <<'PY'
from pathlib import Path
home = Path.home()
root = home / ".cargo" / "registry" / "src"
if root.exists():
for p in root.rglob("wait_timeout-*/src/*.rs"):
print(p)
PYRepository: BumpyClock/gpui-component
Length of output: 155
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Cargo.lock entries ---'
rg -n -A3 -B2 'name = "wait-timeout"|name = "wait_timeout"' Cargo.lock || true
printf '\n%s\n' '--- local cargo registry source for wait-timeout ---'
python3 - <<'PY'
from pathlib import Path
home = Path.home()
root = home / ".cargo" / "registry" / "src"
if not root.exists():
print("missing", root)
raise SystemExit(0)
hits = list(root.rglob("wait_timeout-*/src/*.rs"))
for p in hits[:20]:
print(p)
print(f"count={len(hits)}")
PY
printf '\n%s\n' '--- exact source snippets if found ---'
python3 - <<'PY'
from pathlib import Path
home = Path.home()
root = home / ".cargo" / "registry" / "src"
if not root.exists():
raise SystemExit(0)
for p in root.rglob("wait_timeout-*/src/*.rs"):
txt = p.read_text(errors="ignore").splitlines()
for i, line in enumerate(txt, 1):
if "wait_timeout" in line or "waitpid" in line or "wait_with_output" in line:
start = max(1, i-3)
end = min(len(txt), i+6)
print(f"\nFILE {p}")
for j in range(start, end+1):
print(f"{j}: {txt[j-1]}")
break
PYRepository: BumpyClock/gpui-component
Length of output: 553
🌐 Web query:
wait-timeout 0.2.1 unix.rs waitpid Some(status) reaps child ChildExt implementation
💡 Result:
The wait-timeout crate (version 0.2.1) provides a cross-platform implementation for waiting on a child process with a specified timeout [1][2]. It defines a ChildExt trait that extends the standard library's std::process::Child type with the wait_timeout method [1][3][4]. On Unix-like systems, the implementation in unix.rs is complex because the standard waitpid system call does not support timeouts directly [1][4][5]. To address this, the crate uses a strategy involving SIGCHLD signal handling [1][4]. When wait_timeout is called, the crate manages registration of a signal handler and global state to track the child process's lifecycle without interfering with other threads or existing signal handling (though it notes that bugs may arise if the application is also manually handling SIGCHLD) [1][6]. The core wait_timeout method performs the following: 1. It takes the stdin of the child process if it is present, effectively closing it [1][7]. 2. It calls an internal imp::wait_timeout function [1]. 3. On Unix, this implementation loops while checking the process status, utilizing signals or polling mechanisms to respect the provided Duration [8]. 4. It returns an Ok(Some(ExitStatus)) if the child exits within the timeout period, or Ok(None) if the timeout expires without the child exiting [1][3][4]. This allows users to non-blockingly wait for a process to terminate, enabling patterns like sending a kill signal if the child does not exit in time [4][7]. The implementation is distinct from the Windows version, which leverages the simpler WaitForSingleObject API [1][4].
Citations:
- 1: https://docs.rs/wait-timeout/latest/src/wait_timeout/lib.rs.html
- 2: https://docs.rs/crate/wait-timeout/0.2.1
- 3: https://docs.rs/wait-timeout/latest/wait_timeout/trait.ChildExt.html
- 4: https://alexcrichton.github.io/wait-timeout/wait_timeout/index.html
- 5: https://stackoverflow.com/questions/282176/waitpid-equivalent-with-timeout
- 6: https://docs.rs/wait-timeout/latest/wait_timeout/index.html
- 7: https://alexcrichton.github.io/wait-timeout/src/wait_timeout/lib.rs.html
- 8: On Windows, timeouts are capped at u32::MAX milliseconds alexcrichton/wait-timeout#23
🌐 Web query:
wait-timeout 0.2.1 unix.rs Some(status) child already reaped wait_with_output second wait ECHILD
💡 Result:
The error you are encountering, typically manifesting as an ECHILD error during a wait-related operation on a child process, occurs because the operating system (specifically on Unix-like systems) has already reaped the child process, making it impossible to wait on it again [1][2][3]. In the context of the wait-timeout crate and Rust's std::process::Child, this issue arises for the following reasons: 1. Child Already Reaped: Once a child process has been waited on or reaped (via wait, waitpid, or similar mechanisms), its exit status is collected and the process entry is removed from the system process table [1][4]. Subsequent attempts to call wait or wait_timeout on the same Child object will fail because the kernel no longer tracks that process ID as a child of the current process, resulting in the ECHILD error [1][2][3]. 2. Conflicting Wait Mechanisms: If your application or another library has already waited on the process, or if you have configured the parent process to automatically reap children (e.g., via SIGCHLD = SIG_IGN), the kernel will clean up the child process automatically [3]. When your Rust code then attempts to wait on it, the kernel returns ECHILD because there is no such child to wait for [2][3]. 3. wait_with_output Limitations: The std::process::Child::wait_with_output method waits for the child to exit and collects output [5]. If the child has already been waited on or closed in another thread or by a signal handler before this method is called, it may experience inconsistent behavior or errors [6]. Recommendations for fixing this: - Ensure each process is waited on exactly once [6][1]. - Verify that no other parts of your application (or signal handlers) are using waitpid or setting SIGCHLD to SIG_IGN, which interferes with Rust's child process management [2][3]. - If you are using the wait-timeout crate, it is designed to be used as the primary wait mechanism. Avoid calling wait on the same Child instance elsewhere if you intend to use wait_timeout [6]. - For complex applications, use proper synchronization or a centralized supervisor to handle child process lifetimes, ensuring only one component is responsible for reaping specific child processes [3].
Citations:
- 1: https://doc.rust-lang.org/1.80.1/std/process/struct.Child.html
- 2: https://gtk-rs.org/gtk-rs-core/stable/0.14/docs/glib/source/fn.child_watch_source_new.html
- 3: https://evilpiepirate.org/forge/kent/consciousness/commit/0e459aae9293d0208cc9fc80ba387a36657d7b66
- 4: https://doc.rust-lang.org/nightly/std/process/struct.Child.html?gotosrc=8248
- 5: https://doc.rust-lang.org/std/process/struct.Child.html
- 6: https://docs.rs/wait-timeout/0.1.5/src/wait_timeout/lib.rs.html
Avoid re-waiting the child after wait_timeout returns Some(status) wait_with_output() waits on the same process again, so a fast successful cargo info can fall back to "unknown" instead of "published". Use the returned status directly and read the existing stderr pipe instead.
🤖 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 `@xtask/src/main.rs` around lines 1862 - 1889, Update registry_probe to use the
process status returned by wait_timeout when it returns Some(status), avoiding a
second wait via wait_with_output. Read the existing stderr pipe separately, then
pass status.success() and the collected stderr to registry_probe_status;
preserve the timeout and error paths returning "unknown".
1 participant
Summary
bumpyclock-gpui =0.1.0and companion crates at immutable commitd1a6019136eb9fb9386eaa047a7445c1032d8668Validation
cargo fmt --all -- --checkcargo metadata --lockedcargo test --locked -p xtask— 22 passedcargo clippy --locked -p xtask --all-targets -- --deny warnings./script/clippy --lockedcargo check --locked --workspace --all-targetscargo test --workspace --all-targets --lockedcargo test --workspace --doc --lockedcargo xtask compatibility checkcargo xtask release-checkDeferred gates
Crates.io publication is intentionally out of scope. Strict registry readiness remains blocked until package identities, non-inherited GPUI root patches, and remaining legal/platform evidence are resolved.
Note
Stabilize framework phase zero by pinning GPUI fork packages and adding xtask release validation
bumpyclock-gpui,gpui_platform,gpui_macros,sum_tree) at revd1a6019136eb9fb9386eaa047a7445c1032d8668with exact versions=0.1.0in Cargo.tomlxtaskcrate (xtask/src/main.rs) with subcommandscompatibility generate,compatibility check,publish-plan, andrelease-checkto validate compatibility metadata and publication prerequisitescargo xtask; the release workflow now runs validation instead of publishing directlycargo publishis no longer run in the release workflow; the release gate only validates prerequisites, blocking publication until GPUI packages are on crates.ioMacroscope summarized c83d8c0.