Skip to content

fix: just check failures on macOS - #8426

Merged
kixelated merged 9 commits into
mainfrom
quest/m0/macos-check
Nov 11, 2026
Merged

kixelated merged 9 commits into
mainfrom
quest/m0/macos-check

Conversation

@kixelated

@kixelated kixelated commented Nov 10, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes the three just check failures seen on macOS hosts (moq-dev/moq#8392, moq-dev/moq#8394) and narrows quest/m0/macos-check.md to the two macOS failures left.

Changes

  • doc re-export link (doc/.vitepress/api/index.ts): generate now realpaths root and every package dir. TypeScript reports sources by real path, so a symlinked root (macOS tmpdir() is /var -> /private/var) made dirs miss and links render as ../../private/var/.... Reproduced on Linux with TMPDIR pointed at a symlink: 2 failures before, the re-export test passes after (one unrelated 5s test timeout under host load).
  • carla bridge docs (demo/carla/bridge/src/lib.rs): off Linux the crate is empty, so its //! intra-doc links dangle. Allow rustdoc::broken_intra_doc_links only off Linux, with a reason (clippy's allow_attributes_without_reason). Holds whether or not carla-workspace moves the bridge out of the root workspace.
  • release-plz version (flake.nix, sh/version.py): the dev shell left release-plz out on Darwin ("marked broken"), so macOS fell back to a stray ~/.cargo/bin 0.3.149. The pinned nixpkgs no longer marks it broken and cache.nixos.org has 0.4.1 for aarch64-darwin, so the flake now supplies it on every platform (nix develop --command release-plz --version prints 0.4.1 on macOS); that is what makes the 0.4.1 floor hold there. A cut also fails loud on release-plz older than 0.4.1 (inside the restore guard, so nothing is left half-edited) instead of silently leaving versions unset.
  • Cancelled descendants on macOS (sh/local.ts): the supervisor forwards SIGTERM to its child; on Linux a subreaper keeps the grandchildren in the tree for --no-orphans, but macOS has none, so once the child died its grandchild reparented to launchd, kept stdout open, and hung sh/local.test.py (reproduced). Off Linux, the supervisor now snapshots its descendant tree when cancellation arrives and SIGKILLs the survivors after the child exits, only those whose ps start time still matches the snapshot, so a reused PID is never killed.
  • Relay profiling off Linux (bench/run.sh, bench/profile.test.py): --profile refuses at setup, after validating the selection and before creating evidence, instead of inside run_profiles. The orchestration tests and the /proc SIGUSR1 test skip off Linux; a new test checks the refusal there.
  • More macOS tempdir / Linux-only docs (rs/moq-vaapi/src/lib.rs, sh/gh/gst-formula.test.py): same two causes as above, surfaced once the supervisor change selected the whole tree.
  • JS coverage summary (sh/js/coverage.py): same symlinked-temp-dir cause as the doc fix; report resolves root so it matches the real paths of measured sources. Surfaced by the macOS run.

Choices made without asking: 0.4.1 is the floor because it is the only version known good; the exact version where set-version became correct was not bisected.

Decisions

  • Remaining macOS test gh failures: ✅ widen this PR (rejected: land as is with follow-ups). Maintainer, 2026-11-10: the quest's Goal is just check passing on macOS.
  • Hanging cancellation test: ✅ fix the supervisor at the root (rejected: disable on Darwin).
  • Remaining check cpp and check py: ✅ land this PR and keep quest/m0/macos-check.md open, rewritten to those two with their evidence (rejected: widen this PR again). Maintainer, 2026-11-10.

Verification

  • Linux: sh/version.test.py (7 OK), doc API test under symlinked TMPDIR, biome, ruff, quest check.
  • macOS (aarch64-darwin, maintainer host), quest gate nix develop --command just check, no processes killed by hand: the supervisor change selected the whole tree. All failures in this PR's scope pass (sh/local.test.py, bench/profile.test.py, sh/version.test.py, sh/coverage.test.py, sh/gh/gst-formula.test.py, carla and moq-vaapi docs). Two failures remain in code this branch does not touch:
    • check cpp: clang++: error: -Wl,-lintl: 'linker' input unused [-Werror,-Wunused-command-line-argument].
    • check py: Sphinx py/moq-rs/docs/index.md:147: WARNING: autosummary: stub file not found 'moq.media.catalog' (warnings are errors).

Caused by

  • No single PR: macOS symlinked temp dirs and a stray PATH binary; the carla cfg dates from the crate's creation.

Public API: none. Wire: none.

Merged main on 2026-11-10: it deleted sh/drafts/publish.test.sh (moq-dev/moq#8441), so this PR's fix to it was dropped; sh/local.test.py passes again on macOS after the merge.

Follow-ups

  • check cpp and check py on macOS: tracked by the narrowed macos-check.

Models

Size: S
Implementer: Claude Code, claude-opus-5-5, unknown effort
Implementer: Claude Code, claude-opus-5-5, low effort (iterate, S)

(Written by Opus 5.5)

Review

Rewritten by quest review after each round.

  • Reviewer: Codex, gpt-6.1-sol, medium effort
  • Session: 01a1c88d-2cc5-7fc1-8f2e-7a3d0562bd4d, round 3
  • Head: d05f420
  • Base: origin/main at c54e5ea
  • Score: 9/10, The new head merges base changes without altering the previously reviewed implementation, so no actionable findings remain; builds and tests were not run as instructed.
  • Findings: none

kixelated and others added 3 commits November 10, 2026 01:12
Quest-Claim: d69d9c2ee0c061b2
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title just check passes on macOS fix: just check failures on macOS Nov 10, 2026
… under symlinked tmp

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated and others added 2 commits November 10, 2026 18:52
…setup off Linux

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ula tests

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated marked this pull request as ready for review November 11, 2026 03:02
@coderabbitai

coderabbitai Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor
Pre-merge checks: Inconclusive 2

Failed Failed checks: 2 inconclusive

Check name Status Explanation Resolution
Title check Pending This assessment has not completed. The check will be retried on the next pre-merge check run.
Description check Pending This assessment has not completed. The check will be retried on the next pre-merge check run.
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

kixelated and others added 2 commits November 10, 2026 19:03
…; quest: keep macos-check for cpp and py

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Automated review by review (OpenAI)

Reviewed commit 843a47b.

No actionable findings in the changes reviewed. Real-path normalization aligns the documentation and coverage lookups with their source paths; the release-plz version floor is inside the rollback guard and the dev shell now supplies it on Darwin. The non-Linux cancellation snapshot addresses descendants reparenting after the direct child exits, while leaving Linux subreaper behavior intact. Profiling refusal now occurs before evidence creation, and the platform-specific doc/test adjustments match their Linux-only dependencies.

Overall direction: the fixes are coherent, but this is not a clean macOS check result: the PR reports remaining C++ linker-flag and Python autosummary failures, and GitHub currently reports merge conflicts.

Verification limits: GitHub-only static review of all 14 changed files and related supervisor tests, launcher, version rollback, documentation generation and coverage code. No builds, tests, Nix evaluation or macOS process execution performed; reported validation was not independently reproduced.

@kixelated
kixelated added this pull request to the merge queue Nov 11, 2026

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No critical issues — two small gaps worth a look, both about work this change leaves unverified.

Reviewed changes

Reviewed all 6 commits (843a47b, base main c54e5ea): the macOS just check repairs and the quest deletion that finishes quest/m0/macos-check.md.

  • Supervisor cancellation off Linux (sh/local.ts): a ps -A -o pid=,ppid= snapshot records the descendant tree when a cancellation signal arrives, and survivors are SIGKILLed after the child exits. Linux keeps the subreaper path.
  • Doc API real paths (doc/.vitepress/api/index.ts): generate realpaths root and each package dir so relative(root, dir) matches TypeScript's real-path sources behind macOS's /var -> /private/var.
  • release-plz in the dev shell (flake.nix): moved out of the !isDarwin list. Confirmed against the pinned nixpkgs rev d357a2b — release-plz is 0.4.1 there with no meta.broken and a Darwin curl input, so the removed "marked broken" comment was stale.
  • 0.4.1 floor at cut time (sh/version.py): a Rust cut probes release-plz --version inside the restore guard instead of silently leaving versions unset.
  • Relay profiling refuses at setup (bench/run.sh, bench/profile.test.py): the uname -s check moved into the --profile arm after selection validation and before ARTIFACTS/RUN and the "Benchmark evidence" print, plus the matching test and off-Linux skips.
  • Real temp paths and Linux-only docs: pwd -P in sh/drafts/publish.test.sh, Path(tmp).resolve() in sh/gh/gst-formula.test.py, root.resolve() in sh/js/coverage.py, and cfg_attr(not(linux), allow(rustdoc::broken_intra_doc_links)) in demo/carla/bridge/src/lib.rs and rs/moq-vaapi/src/lib.rs.

ℹ️ The two remaining macOS failures lose their quest with this one

The PR body keeps check cpp (-Wl,-lintl reaching a compile-only clang++ under -Werror) and check py (missing Sphinx autosummary stub for moq.media.catalog) as follow-ups, but nothing under quest/ records them once quest/m0/macos-check.md is deleted. A PR body is not a tracked commitment here, and both outlive this branch.

Technical details
# Two macOS `just check` failures are untracked after this deletion

## Affected sites
- `quest/m0/macos-check.md` — deleted, and its `quest/m0/README.md` entry removed.
- `quest/m1/carla-workspace.md` — edited to drop the now-dangling cross-link.
- `quest/` — no file mentions `-lintl` or `moq.media.catalog`.

## Required outcome
- The two failures the quest's Goal does not cover remain findable in the quest tree, so a later
  `/quest-complete` or `/quest-audit` run can pick them up rather than rediscovering them.

## Suggested approach (optional)
- File one or two follow-up quests (or add a line to `quest/m0/nightly/macos.md`, which already
  owns macOS-host coverage) naming each failure and its owning check.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

Comment thread sh/version.py
# Older release-plz, such as a non-Nix one ahead on PATH, leaves versions unset without failing.
found = run(root, "release-plz", "--version").split()[-1]
if version(found) < (0, 4, 1):
raise ValueError(f"release-plz {found} is older than 0.4.1; use the Nix dev shell's")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The floor itself ships untested, and the existing fake release-plz in sh/version.test.py:258 (a shell script that corrupts Cargo.toml/Cargo.lock and exits 1 on any argument) now fails on this --version probe rather than inside set-version, so the test no longer exercises what its comment claims. Since CONTRIBUTING.md leans on that fixture to catch release-plz behavior changes, give the fake a --version answer of release-plz 0.4.1 and add a case with an older one asserting this refusal.

Technical details
# The release-plz floor has no test, and the restore fixture moved its failure point

## Affected sites
- `sh/version.py:248-251` — the new probe and refusal.
- `sh/version.test.py:258` — fake binary is argument-blind; the `--version` probe trips it first.
- `CONTRIBUTING.md:137` — states the fixture catches release-plz upgrades that break the cut.

## Required outcome
- A test proves an older release-plz fails the cut loudly with the versions untouched.
- `test_cut_refuses_bad_targets_and_restores_partial_external_failure` still fails *inside*
  `set-version`, after release-plz has edited manifests.

## Suggested approach (optional)
- Make the fake print `release-plz 0.4.1` when `$1 == --version` and keep the corrupting failure
  otherwise; add a sibling fake printing `0.3.149` to cover the refusal.
- The error text reads truncated at `use the Nix dev shell's`; name the binary, e.g.
  `release-plz {found} is older than 0.4.1; run the cut inside `nix develop``.

Comment thread sh/local.ts Outdated
for (const name of ["SIGINT", "SIGTERM", "SIGHUP"] as const) {
process.on(name, () => {
signaled ??= 128 + constants.signals[name];
if (process.platform !== "linux") for (const pid of descendants(process.pid)) tree.add(pid);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This branch is the only new logic in the PR and nothing in CI runs it: Linux skips it, and platform.yml's macOS job only runs just rs macos / just rs macos-test, never sh/local.test.py. Taking the snapshot on every platform would make the existing Linux test_cancellation_reaches_descendants_and_waits_for_cleanup cover the parse-and-kill path, since the sweep is a harmless superset of what the subreaper and --no-orphans already do.

Technical details
# The non-Linux descendant sweep is unexercised, and still gaps on normal exit

## Affected sites
- `sh/local.ts:22-37` — `descendants()` walk, platform-independent apart from its caller.
- `sh/local.ts:44-58` — sweep gated on `process.platform !== "linux"` and on a signal.
- `sh/local.test.py:210` — `test_exit_reaches_descendants_orphaned_by_their_parent` stays
  Linux-only, so a child that exits normally leaving a stdout-holding grandchild still hangs
  off Linux; only the cancellation trigger is covered by the new code.

## Required outcome
- Some automated lane exercises the snapshot/kill path (Linux CI is the only option today).
- Decide whether the normal-exit orphan case is in scope for this PR or a follow-up; if left,
  say so, since that is the same hang with a different trigger.

## Suggested approach (optional)
- Drop the platform guard so the tree is recorded on Linux too; the two cancellation tests in
  `sh/local.test.py` then cover the new code, and the resulting behavior is one path rather
  than two.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No new issues in these commits — the two inline suggestions from the previous review still stand.

Reviewed changes

Reviewed the delta since the last review (843a47b → d05f420, 3 commits): the start-time hardening in the supervisor, the kept-and-narrowed quest, and the merge of main.

  • Reused PIDs are no longer killed (sh/local.ts): one ps -A -o pid=,ppid=,lstart= snapshot now carries each PID's start time through descendants(), and the post-exit sweep re-reads the table and SIGKILLs only a recorded PID whose start time still matches.
  • The quest survives, narrowed (quest/m0/macos-check.md, quest/m0/README.md): only check cpp (-lintl reaching a compile-only clang++ under -Werror) and check py (missing moq.media.catalog autosummary stub) remain, each with its recorded failure text, the maintainer's narrowing decision, and undiagnosed-cause leads. This resolves the untracked-follow-up point from the previous review.
  • main merged in: imports the quest changes from #8465/#8451/#8418 (quest/m0/ietf-max-age.md and its README line) and drops this branch's sh/drafts/publish.test.sh fix, since main deleted that file; the body records that sh/local.test.py passes again on macOS after the merge.

The new sh/local.ts logic reads correctly: lstart is stable for a live process, both snapshots parse it the same way, and a reused PID (or one that exited) fails the string comparison instead of being killed. The comment on sh/local.ts:47 about the non-Linux sweep having no lane that runs it still applies to this code, and the sh/version.py:251 note about the untested 0.4.1 floor is unchanged, so neither is repeated inline here.

Pullfrog  | Fix it ➔ | View workflow run | Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

Merged via the queue into main with commit e565f10 Nov 11, 2026
8 of 9 checks passed
@kixelated
kixelated deleted the quest/m0/macos-check branch November 11, 2026 04:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant