fix: make the codex packaging and SDD skill test suites pass reliably off-macOS#1994
Merged
Conversation
…dtar
The packaging pipeline only worked on a Mac with default umask, for
three stacked reasons:
- The deterministic-metadata tar flags (--uid/--gid/--uname/--gname)
are bsdtar spellings; GNU tar rejects them, so the tar.gz archive
step died on Linux. Detect the tar flavor and use --owner=:0
--group=:0 --numeric-owner on GNU tar, which writes byte-identical
ustar headers (uid/gid 0, empty uname/gname).
- Staged file modes depended on two umasks canceling out: git archive
masks entry modes with tar.umask (git default 0002 -> 775), and the
unflagged tar extraction re-masked with the process umask (022 on
macOS -> 755, but 002 elsewhere -> 775). Pin tar.umask=0022 on the
archive call and extract with -p so staged modes are canonical
755/644 on every machine.
- The test's timestamp assertion parsed bsdtar's -tv column layout and
expected epoch 0 rendered in a US timezone ("Dec 31 1969"); GNU tar
uses different columns and UTC hosts render "1970-01-01". Assert
mtime == 0 via python3 tarfile instead, matching how the test
already checks zip timestamps.
tests/codex/test-package-codex-plugin.sh now passes on Linux/GNU tar;
the bsdtar branch preserves the exact flags that passed on macOS.
tests/claude-code/test-subagent-driven-development.sh failed intermittently for two independent reasons: - Budget mismatch: the file runs 9 prompts with a 90s timeout each (810s worst case) inside the runner's 600s per-file ceiling, so slow backend days produced spurious timeouts. Raise the runner default to 900s and fix the help text, which claimed the default was 300. - Case-sensitive prose matching: the assert helpers grepped free-form model output case-sensitively, but models capitalize the skill's own headings — observed failures include "Do Not Trust the Report" missing pattern "not trust" and a structured answer missing "First:.*spec.*compliance". Match case-insensitively in assert_contains/assert_not_contains/assert_count/assert_order, widen two Test 5 keyword patterns to phrasings observed in real runs, and make assert_order dump the output on failure the way assert_contains already does, so the next flake is diagnosable. Observed 3 failures across 4 runs before the change (timeout, two distinct pattern misses); 3/3 consecutive full runs pass after it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Who is submitting this PR? (required)
What problem are you trying to solve?
Running the full test matrix on dev (Linux, GNU tar, umask 002, TZ=UTC) during pre-release verification of #1993 turned up two failing suites, both failing on clean dev with no local changes:
tests/codex/test-package-codex-plugin.shfails hard on any Linux box.tar: unrecognized option '--uid'— the packaging script's deterministic-metadata flags are bsdtar spellings that GNU tar rejects, so the tar.gz archive step dies and three assertions cascade. Two more portability bugs hide behind it: staged file modes only came out 755 because git archive'star.umaskdefault (0002 → 775) happened to be re-masked by macOS's default process umask (022 → 755) — on a umask-002 Linux box the executable lands as 775 and the mode assertion fails; and the test's timestamp assertion parses bsdtar's-tvcolumn layout expecting epoch 0 rendered as "Dec 31 1969", which is both the wrong column split for GNU tar and the wrong string on any UTC host.tests/claude-code/test-subagent-driven-development.shflakes. Observed 3 failures across 4 runs, each different: a spurious timeout (the file's worst case is 9 prompts × 90s = 810s, but the runner kills files at 600s), and two prose-matching misses where the model capitalized the skill's own headings — output containing "Do Not Trust the Report" failed the case-sensitive patternnot trust, and a structured answer failedFirst:.*spec.*compliance.What does this PR change?
Codex packaging: detect the tar flavor and use the GNU spellings (
--owner=:0 --group=:0 --numeric-owner, byte-identical ustar headers) when tar is GNU; pin-c tar.umask=0022on thegit archivecall and extract the stage with-pso modes are canonical 755/644 regardless of builder umask; assert tar mtimes via python3tarfile(mtime == 0), matching how the test already checks zip timestamps. SDD test: raise the runner's per-file budget to 900s (and fix its help text, which claimed a 300s default that was never true), make the assert helpers match case-insensitively, widen two Test 5 keyword patterns to phrasings observed in real runs, and makeassert_orderdump the model output on failure the wayassert_containsalready does.Is this change appropriate for the core library?
Yes — it fixes the repo's own test infrastructure and release packaging script. No behavior-shaping skill content is touched.
What alternatives did you consider?
chmod -Rnormalization of the stage — rejected as a workaround; the root cause was two umasks accidentally canceling, and pinningtar.umask+-premoves both dependencies instead of papering over them.Does this PR contain multiple unrelated changes?
Two fixes, one per commit, deliberately bundled: they are the two failures from the same full-matrix test sweep, and the maintainer asked for them as one PR. Happy to split if preferred on review.
Existing PRs
TZ=UTCfix already landed, and this PR's python-tarfile assertion removes the tar-side TZ dependence it identified. Allow Codex packaging from linked worktrees #1910 (closed) — a different packaging portability bug (worktree guard), deliberately NOT included here; it remains open work. Both were closed for batch-submission reasons, not correctness; this PR credits the underlying reports.Environment tested
macOS/bsdtar was not available to re-test; the bsdtar branch keeps the exact flags that passed there before this change, and the GNU branch was verified to write byte-identical ustar owner headers (empty uname/gname, uid/gid 0) by comparing raw header bytes.
New harness support
Not applicable — no new harness.
Evaluation
tar: unrecognized option '--uid', then the mode assertion at 775 vs 755); passes completely after, with the marketplace-manifest and codex-plugin-sync suites re-run green alongside.not trustcase miss;First:.*spec.*compliancemiss). After: 3/3 consecutive full runs pass (each run is 9 realclaude -psessions). The case-insensitivity fix was also validated retroactively against the captured failing outputs — both recorded pattern misses match undergrep -i.Rigor
superpowers:writing-skillspressure testing applicable; no behavior-shaping content touched.Human review