Repository navigation
fix(desktop): isolate process tests and close unlisted Apple child descriptors - #846
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@codex review |
Codex Review Summary: ✅ CompletedThis 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. |
|
Codex Review: Didn't find any major issues. You're on a roll. 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". |
🛡️ Codex Security Review · Automatically triggeredSecurity 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. |
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. 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". |
|
Ready for lead native qualification: 3be0b4f (HACK-2243). Not acceptance/merge-ready yet. Both required Env copy fixes are delivered at bb70d35; #818 (comment) . This is the next item in the lead's order. #846 uses the existing pinned nextest 0.9.151 installer, per-test processes and zero retries in both desktop jobs and apps/desktop/scripts/test.sh; the wrapper makes its own HOME/HACK_HOME. Executable fixtures are flushed, closed, chmodded, then atomically published. TESTING.md distinguishes what ENOENT, missing ready markers, socket WouldBlock, and delayed pipe EOF do and do not prove. The precise historical ENOENT cause is not claimed proven. The product Apple CLI boundary uses absolute posix_spawn, captured child PATH/environment overrides, CLOEXEC_DEFAULT and explicit stdio/group/cwd actions. SIGPIPE resets to default, and the child mask is explicitly empty (deliberate policy, not Rust parity). Regressions: intentionally inheritable fd with old-Command positive control; stdio/argv/env/cwd/pgroup; blocked worker SIGTERM with scoped mask restoration. Linux keeps std Command; its inherited signal mask remains a non-blocking follow-up, avoiding an unrelated Linux fork-path policy change in this Apple descriptor fix. External spawners not using CLOEXEC_DEFAULT can still inherit the brief initial pipe/fcntl window; this boundary cannot alter GPUI/other libraries' spawning code. Evidence: local Linux pinned nextest workspace 76/76, no retries/skips; fmt, both Clippy/build modes; Apple-target core/tests Clippy with warnings denied (type-check only). Exact-head Codex clean #846 (comment) ; same persistent Claude CLEAN, claude-2243-review-r2.md; CLEAR at 3be0b4f. Hosted Linux desktop/scope/secrets/docs pass. Hosted macOS queued; general CI still incomplete. PR marked ready for review so draft fan-ins do not masquerade as code failures. Please land #843 independently first (valid and Mac-batch verified); #846 does not close/supersede it. Then merge next into #846 and requalify the combined exact head. Native command, from apps/desktop after installing the existing pinned nextest: HACK_DESKTOP_TEST_REPETITIONS=200 bash scripts/test.sh . The wrapper runs all workspace tests each iteration then doctests. This final combined head needs the lead's 200 consecutive Mac suites and three consecutive green hosted macOS desktop jobs; neither has been claimed. No real home, daemon, Docker, services or GUI effects are needed. No merge performed. |
🛡️ Codex Security Review · Automatically triggeredSecurity 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. |
|
Native qualification progress at unchanged 3be0b4f: hosted macOS desktop job 132447179259 in run 43594890603 passed, including the new native tests. This is green 1 of the required 3 consecutive hosted jobs. Requested a rerun of that macOS job for green 2; no native-200 or full-CI claim. General CI failures reported by the watcher belong to superseded cancelled run 43594890559; replacement 43595128254 is queued. |
|
Lead Mac qualification at 🤖 Generated with Claude Code |
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! 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". |
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 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". |
|
Ready for lead: e2c6cc6, for the HACK-2243 200-run Mac rerun. This supersedes the 3be0b4f qualification note; it is not acceptance or merge-ready evidence. The remaining fixture failure is addressed in both source fake-daemon callers and the headless-state fixture. They share one 60-second accept/read/write conversation deadline. The request reader first restores blocking mode: Darwin/BSD can inherit O_NONBLOCK from a nonblocking listener, making errno 35 immediate rather than evidence that a read timer elapsed. WouldBlock is handled only within the deadline; complete headers are bounded to 4096 bytes. No short fake-server read timer remains. The new regression uses a nonblocking listener, forces accepted nonblocking mode on Linux too, sends a delayed split request beyond the former two-second timeout, and asserts O_NONBLOCK was cleared. Product code/timeouts and nextest retries are unchanged by these two follow-ups. Local pinned nextest workspace: 73 unit + 4 integration tests passed, zero skipped/retries; fmt, default all-targets Clippy and Apple-target core/test Clippy with warnings denied passed. Prior both-feature Clippy/build evidence remains applicable to unchanged product/UI code. Same persistent Claude CLEAN at this exact head, round 4. Exact-head Codex clean: #846 (comment) . Review-status CLEAR at e2c6cc6, no unresolved bot threads. The lead's prior 124 consecutive 79/79 suites then one failure at repetition 125 on 3be0b4f is improvement, not a 200-pass result. No previous NotFound/never-started symptoms occurred in that batch. That evidence and old hosted greens do not qualify this new revision. Please rerun from apps/desktop: Hosted status on this head: Linux desktop passed; macOS desktop queued. The three consecutive hosted macOS greens must be collected on this revision. No full-CI-green claim. |
🛡️ Codex Security Review · Automatically triggeredSecurity 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. |
|
Lead Mac qualification at 🤖 Generated with Claude Code |
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 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". |
🛡️ Codex Security Review · Automatically triggeredSecurity 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. |
|
Ready for lead: 14c0d3a Takeover owner (Claude). No new commits since the previous owner's merge.
Not verified by me: macOS. Your 200/200 receipt is for e2c6cc6; this merge brings in next's changes, so a short Mac repeat at this head is your call. 🤖 Generated with Claude Code |
Adds the names-only Env tab: read on open, project change and Check again, missing required names first, locked/unknown key readability, and explicit empty versus unreadable states. Uses `hack env list --json --path <root>` without a value flag; exit 1 still accepts a bare report. Typed models discard values/materialized fields and raw diagnostics. Generations drop stale selections; marker/discovery guards refuse missing or ancestor-shadowed roots. Canonical selected-scope absence differs from project-wide absence; empty legacy scope arrays show global. Visible copy explains that key readability can be unknown and that Set indicates storage, not confirmed readability. The locked state shows `hack doctor` beside **Copy command**. The available app/core tabs and fixtures contain no internal request/ticket IDs outside source comments; the core always-on tooltip and README are aligned. No native Env, key handling, badge styling or upstream CLI-shape changes. Based on `next`; core warmups, ordered snapshots, log-tail invalidation and recovery gates are preserved. Local Linux at **08db90a75609b1fea06791eeb9847b95949ec55e**: Rust 1.98.1, own target, jobs 2; fmt, 77 unit + 6 integration tests, both Clippy modes with warnings denied, both builds and privacy pass. Eight affected Env light/dark screenshots were rendered and inspected at the copy-fix head bb70d35; the subsequent frozen-next merge changes no UI/fixtures and retains those receipts. Loading/error renders are retained unchanged. Fake CLI/temp HOME/HACK_HOME/autostart disabled; no real env command, owner registry/daemon, Docker, sudo or GUI window. Normal merge of frozen next 8e7242d is pushed at 08db90a. README preserves Env docs and incoming process-retirement test guidance; runner tests match next/843. No app/Env read logic changed. Root and desktop scripts/test.sh are absent, so pinned own-target Cargo checks were used; no unmerged 846 wrapper was imported. Exact-head Codex and SAME persistent Claude r5 are clean; review-status CLEAR. Both hosted desktop jobs and successful replacement CI selected shards/fan-in pass. **[Final Ready for lead](#818 (comment) at frozen 08db90a supersedes all earlier holds; superseded cancellation excluded. Absolutely no more pushes. No PR merge during next.17 freeze; lead only after publication. Lead's macOS build/renders/state pass covers the previous 609d5cc head; new copy has fresh Linux offscreen evidence and the frozen merge head has hosted Linux/macOS desktop success; runtime limitations remain. Separately tracked macOS spawn flakes are not changed here. No full-CI-green, merge or release claim. Limits: real env CLI rejects native roots, so native Env is Unreadable pending CLI support; fake native tests prove backend selection/parser behavior only. macOS keychain prompting remains unverified. Reads are bounded but not deduplicated; stale results are discarded. Marker pre/post checks are not an atomic filesystem fence. No merge or watcher by this unit.   Freeze branch-update order: #846 → #818 → #808. Env MERGE PUSHED is complete, so Logs can be NEXT. Preserve shared tab/fixture/screenshot/source hooks. No PR merges during freeze; lead only after publication. No new shape adoption or scope. --------- Co-authored-by: Dimitri Kennedy <344842+roodboi@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Threaded macOS desktop tests intermittently failed while writing/spawning fake CLI scripts or waiting on their sockets/pipes. The desktop test wrapper and both CI jobs now use the existing pinned nextest installer, one process per test, zero retries, and temporary HOME/HACK_HOME. Executable fixtures are closed before atomic publication. TESTING.md distinguishes the reported ENOENT/startup/socket-timeout symptoms and the remaining native evidence needed.
The application also spawns CLI processes concurrently. Rust 1.98.1 on Apple creates pipes before marking them close-on-exec; its default spawn can retain another thread's descriptor in that window. The Apple runner now uses
posix_spawnwithPOSIX_SPAWN_CLOEXEC_DEFAULTand explicit stdio/group/cwd/environment actions. A native regression holds an inheritable descriptor open: a standard-spawn control must inherit it, while the app boundary must close it. Another native test checks the preserved process contract. Linux keeps stdCommand.Local evidence: pinned nextest workspace (76 tests, zero skipped/retries), fmt, both app Clippy modes with warnings denied, both builds, and Apple-target core/test Clippy passed. Apple cross-check is compilation only. The two native regressions, 200 consecutive lead-Mac suites, and three consecutive green hosted macOS jobs remain acceptance gates. No GUI, owner home, real daemon, Docker or network used by tests.
HACK-2243. #843 remains a separate, valid PR for generous process-retirement deadlines; this PR does not supersede or close it. Land #843 first, then integrate next here before final combined Mac qualification. No release workflow, root toolchain/locks, runtime-core or retained native app changes.
Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.