ci: Linux workflows with a gate on unrecorded divergence - #18
Merged
Conversation
The repository had no CI. The conformance suite could compare the two implementations, and nothing ran it. A divergence could reach main and no machine would object. The workflow builds the Node pty at a pinned commit, runs the conformance suite against both binaries, and fails when a difference is not recorded in the ledger. The pin matters. A local Node checkout seven commits behind produced eight false divergences on 2026-09-05, and every one was an artifact of the stale reference. The workflow reads the commit from crates/pty-conformance/node-ref and prints it. check-divergences.py fails in both directions: on a difference nobody has recorded, and on a record that no longer happens. A ledger that only grows stops describing the present. Four controls cover both directions, a flipped side, and an empty result. The ledger is seeded with the two real differences. Both are one Node defect: the Node binary truncates `pty completions fish` when stdout is a pipe, at 123 of 167 lines, and writes all 167 to a file. Formatting and clippy report their counts and do not gate. main does not pass either today: 1093 fmt diffs and 50 clippy warnings. Making either a gate needs a cleanup commit, which is a separate decision from adding CI.
Nathan decided CI is Linux only and pointed at the Node pty's workflows as the model. This replaces the single ci.yml with nix.yml and test.yml, both on ubuntu-latest, both triggered the same way, using the same nix action. The macOS job is gone. The decision and its reason are recorded in the README: proctable.rs carries a macOS process-table reader with hand-declared struct offsets that no Linux job compiles, so an offset or SDK change breaks the Mac build and CI will not notice. That is accepted because the tool runs on two Macs every day, so human use is the detection mechanism. The first run failed on security_fixes::concurrent_stealers_cannot_both_win, which is the flakiness the README already documents: 139 test binaries run in parallel driving real processes, and a runner is a loaded machine. Re-run alone here, that binary passes 8 of 8. So scripts/ci-test-workspace.sh encodes the procedure the README already prescribes. It re-runs only the failing binaries, single-threaded, and fails if they fail again. A real defect still has to lose twice.
myobie
changed the title
The first run timed out. The conformance step needs about 45 minutes on a two-core runner, and I had set a 60 minute job timeout from a 30 minute measurement taken on a many-core machine with a warm cache. The gate was not what failed. My estimate was. Conformance now has its own workflow, so build, lint counts and the workspace tests report in about 12 minutes instead of queueing behind it. Four shards each take a slice of the files and run BOTH binaries on that slice, which keeps the two sides on one runner: this suite races under load, and giving a shard another binary's work is the way to make that worse. The divergence check runs ONCE, over the combined set. It cannot move into the shards. The check fails in two directions, and the second one, a recorded divergence that no longer happens, can only be decided by knowing the divergence appeared in no shard at all. A missing shard is refused rather than treated as agreement. Sharding is the right treatment because the cost is the tests. Measured: five files cost 10.48 s as five cargo invocations and 10.27 s as one, with the tests accounting for 10.21 s. The work parallelises across machines and does not compress on one. That measurement, and the reason a single invocation needs --no-fail-fast, are recorded beside the loop they describe.
Shard 2 hit the 45 minute timeout. It printed its file list and then produced nothing for 41 minutes, so there is no evidence of which file hung. The cause is the pipe to `tail -60`. A pipe buffers until the command ends, and a command killed by a timeout never ends, so the output that would name the file was discarded. The shard now streams through tee, and the log is kept in the artifact whether the shard passes or not. The gate now counts red.txt files rather than directories. A hung shard still uploads its log, so its directory exists while its result does not, and counting directories would read that as a complete set. Three shards passed in about 8 minutes each, doing work one serial job could not finish in 60, so the shape is right. This is only about being able to see the failure.
Every shard failed, including three whose work was perfect. Shard 0 finished 152 of 152 on both binaries with no differences and then exited 1. The cause is the instrumentation I added to diagnose the hang. tee opens its output file when the pipeline starts, which is before conformance-both.sh creates its --out directory, so tee died with "No such file or directory" and set -o pipefail turned a clean run into a failure. It also cost the evidence it was added to collect: shard 2 hung again for the full 45 minutes and there is still no streamed log, because tee never opened one. The mkdir is the whole fix.
Shard 2 hung three times, each stopping at exactly the 45 minute timeout. The streamed log finally showed why: it was not slow and it did not fail. It finished its work in 2 minutes 44 seconds, printed the full table, 172 of 172 on Rust, and then the step sat for another 38 minutes. At cleanup the runner reported: Terminate orphan process: pid (23143) (pty-daemon). The step piped the script into tail, then into tee. A pipe makes completion depend on end-of-file, and end-of-file needs every write end closed, including one held by a process that outlives the run. This suite leaves daemons behind. Writing to a file removes the dependency. It also keeps the partial output when a step is killed, which the pipe into tail did not: that is why the first two hangs left no evidence at all. A heartbeat now names the most recently finished file every 30 seconds, so the next hang of any kind is visible while it happens. The leaked daemon is a separate matter and a real one. A tool that leaves a process holding a caller's inherited descriptor can wedge any pipeline it is used in, long after its own work is done. That is worth its own investigation and it is not fixed here.
The gate failed and it was right three times over. It found an UNRECORDED divergence nobody had written down anywhere: pty-rust sets PWD in the child environment to the directory as the caller wrote it, following node-pty's own env.PWD = cwd, and the Node pty does not set it at all. Where a directory is reached through a symlink the two disagree, so pty run --cwd X answers one way from inside X and another from outside. It is in neither docs/parity.md nor a decision record. It is now in the ledger. It also called both seeded entries STALE, and they were. I measured the Node completions truncation on Node v24.18.0, where it reproduces every time, 123 lines of 167. CI runs Node 22 and does not reproduce it at all: same commit, same test, different answer. So the reference COMMIT is not the only input that decides the answer. The runtime is too, and I had pinned one and not thought about the other. The workflow now says so where the version is set. The defect is real on Node 24 and the Node pty owner has a fix. The entries are removed because a ledger records what CI can see, and kept in the header so the knowledge is not lost with them.
I recorded it as version-dependent, which implies the Node major decides it. The Node pty owner measured the same unfixed binary on arm64 macOS and got the full 12138 bytes under both 22.21.1 and 24.0.2. So runtime, platform and scheduling decide it together. My measurement was one environment: x86_64 Linux, Node v24.18.0, 123 lines of 167 every run. The only claim I can defend is that it does not reproduce where this gate runs. The header now says that, and records their forced delayed-write control: zero bytes against the old exit path, 12138 against the fixed one, on 22 and 24 alike. That control does not depend on scheduling, so it proves the fix on a runtime where the natural failure never appears. Comment only. The recorded entry is unchanged.
The gate failed on two differences that were not differences. A run reported nesting_prevention::restart_force_restores_the_attach and up_down::down_stops_only_named_sessions as Node-side failures. Both files then passed 3 of 3 locally against the same Node binary. They were lost races. The previous single-job workflow re-ran differing files before failing. I dropped that when I split the run into shards, and this is the consequence. The retry belongs in the shard, where the binaries and the toolchain already exist, so it goes there. It re-runs only the differing files, which is both cheap and much less contended than the full shard, and rewrites red.txt. A file that was clean contributes nothing to red.txt either way, so re-running just the differing ones is still a complete result for the shard. A real difference is reproducible and survives. A race has to lose twice. The empty-red.txt guard matters more than it looks: conformance-both.sh with no file arguments runs all seventy, so an unguarded retry on a clean shard would quietly double the work. I hit that while testing this by hand.
The comment said the cost is the tests, from a five-file sample that happened to contain the one seven-second outlier. Across all 140 invocations the reported test time totals 5.1 minutes, no invocation exceeds 12.5 seconds, and the compile before the first result took about 26. The conclusion is unchanged, because sharding parallelises the compile too. The reason was wrong, and a wrong reason in a durable comment sends the next person optimising test runtime, which has almost nothing to give.
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
1 participant
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.
The repository has no CI. The conformance suite can already compare the two implementations, and nothing runs it. Parity is measurable, it is measured well, and it is not monitored.
Two workflows, both
ubuntu-latest, shaped like the Node pty'snix.ymlandtest.ymlso a person moving between the repositories reads one pattern.What gates, and what does not
nix buildand the binary runscargo build --workspace --releasecargo fmt --checkreports 1093 diffs on main and clippy reports 50 warnings (measured 2026-09-05). Neither has ever passed. Gating either means a cleanup commit first, which is a separate decision from adding CI, and a large reformat would collide with in-flight branches. Both run and print their counts into the job summary, so the debt is visible on every run rather than hidden behind a check nobody enabled.The divergence gate
scripts/conformance-both.shwrites the tests whose result differs between the binaries.scripts/check-divergences.pycompares that againstcrates/pty-conformance/divergences.tomland fails in both directions:The second rule is the point. A ledger that only grows becomes a list of claims nobody re-reads, and its entries outlive their reasons. Four controls verify the gate actually fails: an unrecorded rust-red regression, a stale entry, a flipped side, and an empty result against a seeded ledger. A gate nobody has watched fail is a gate nobody knows works.
The Node commit is pinned, and I found out why the hard way
crates/pty-conformance/node-refholds the reference commit; the workflow checks it out and prints it.I learned this requirement by making the mistake. Earlier on 2026-09-05 I measured against a local Node checkout seven commits behind, because I cloned from it instead of fetching. It produced eight divergences, every one an artifact, and I reported them before a peer noticed the hash disagreed. Without a pin, this workflow manufactures exactly that error and somebody records the result as intended.
The pin also makes the ledger stable: bumping
node-refand clearing resolved entries go together, and the gate enforces it. When the Node fix below lands and someone bumps the pin, the two seeded entries become stale and CI says so.The seeded ledger: two entries, one defect
Measured against Node
86dcc5eon 2026-09-05: pty-rust 660/660, the Node pty 658/660. Those two are the entire difference between the implementations.Both are one Node-side defect. The Node binary truncates
pty completions fishwhen stdout is a pipe — 123 of 167 lines piped, all 167 to a file;completions zshthe same. It is Node exiting before an async pipe drains, not a content difference: the checked-in artifacts and pty-rust output are byte identical. Sopty completions fish | source, the normal fish idiom, silently writes a broken file. Reported to the Node pty owner, who has a fix on a branch.Neither is an intended divergence, so the ledger records
kind = "node-defect"rather than calling a bug a decision.Flakiness is handled the way the README already prescribes
The first run of this workflow failed on
security_fixes::concurrent_stealers_cannot_both_win. That is the flakiness the README documents: 139 test binaries run in parallel driving real processes through real PTYs, and a runner is a loaded machine. Re-run alone, that binary passes 8 of 8.scripts/ci-test-workspace.shencodes the README's own rule — "re-run it alone before treating it as one" — by re-running only the failing binaries, single-threaded. A real defect still has to lose twice, once under contention and once alone.What it would have caught this week
Nothing, and I want to be exact rather than let it read scarier.
My earlier answer said it would have caught PR #12 on 2026-09-04. That was wrong, and it was wrong because of the stale reference above: Node shipped the same keep-expiry feature the same day in
a2ae2db. The implementations agree.So this would not have caught a regression this week, because nobody shipped one. What changes is that the next one is caught by a machine on the pull request, instead of by someone running the suite by hand after being asked a question.
macOS is deliberately not covered
Both workflows are Linux. Nothing in CI builds or tests this on a Mac.
crates/pty-core/src/proctable.rscarries a macOS process-table reader that no Linux job compiles: libproc, plus a sysctl fallback with hand-declaredkinfo_procstruct offsets. Offsets are exactly what a new macOS moves. A change there, or an SDK change, breaks the Mac build and CI will not notice.That is accepted, not overlooked: the tool runs on two Macs every day, so a broken Mac build surfaces immediately in use, and human use is the detection mechanism. The trade holds while the daily users are the affected users. The reasoning is recorded in the README so whoever hits that failure finds a decision rather than an oversight, and can revisit it cheaply.
Not in this pull request
docs/conformance.mdwas generated against the older Node and is stale; five Node suites postdate it. The behaviour is covered by existing Rust tests, the pins are not. Regenerating it is follow-up.