Skip to content

Add a long-haul tier, a probation tier, and suites that do only what their claims need - #178

Merged
MikeGrier merged 20 commits into
mainfrom
mikegrier/long-haul-tier
Nov 5, 2026
Merged

MikeGrier merged 20 commits into
mainfrom
mikegrier/long-haul-tier

Conversation

@MikeGrier

Copy link
Copy Markdown
Owner

Adds a long-haul tier and a probation tier, and makes suites do only what their claims need (M45).

Two post-merge sabotage sweeps (c714e83b, 13f06742) failed controls because tests stalled on runners while idle. The fix chosen was to make the suites do less, and to move scale-for-its-own-sake to a daily tier. Decision: DESIGN-NOTES.md; measurements: DESIGN-RATIONALE.md.

Tiers

  • A test is long-haul or probation by an #[ignore = "long-haul: ..."] / "probation: ..." reason and a long_haul / probation module; tools/check-test-tiers.ps1 refuses a disagreement, in CI on every push.
  • .github/workflows/long-haul.yml runs daily when main has moved (plus dispatch). The long-haul tier fails the run; the probation tier does not.
  • Probation outcomes are recorded on a probation-results branch by a separate record job (the only one with contents: write) and reported by tools/report-probation.ps1. The promotion threshold is a working position, not a decision.
  • A tier runs one test at a time (run-tier.ps1 -TestThreads, default 1).

Suites reduced

  • blocking_push capacity test, windows-file-enumeration-sys large-directory tests, and seven win-multi-file-tail-watcher tests run smaller; full-scale forms are long-haul.
  • Cost probes gain --iteration-divisor (feat(platform-probes)), so the census and probe tests run at a fraction of the workload.
  • Four waits that are their claim were measured and left, each with its reason beside the constant.

Tests that never ran

  • Six (five flush-barrier stress instruments, the 4 GiB buffer test) and 17 in windows-file-watcher's stress.rs / scenario_stress.rs are now on probation. The latter returned immediately unless WINDOWS_FILE_WATCHER_STRESS was set, which nothing sets, so they reported as passing without running. All 17 pass when run (run-tier.ps1 -Module probation).

Effect

Serial whole-workspace run: 140 s to 101.6 s; tests of a second or more: 22 (65 s) to 13 (31.5 s). The remaining 13 are each recorded with a reason in COMPLETED-CHECKLIST.md.

Not yet exercised

The long-haul workflow and the record job have never run on a runner; the first scheduled runs are what show whether they work. Parked items M45+.1 (retry/timeout for a timed-out survives sabotage control) and M45+.2 (merge or delete the publisher copy) wait on post-merge sweeps and those runs respectively.

No sabotage sweep was skipped where a manifest names a changed test; tier tooling has its own manifests (sabotage-tiers.json, sabotage-long-haul.json, sabotage-probation.json).

Mike Grier and others added 12 commits November 3, 2026 17:55
…y what their claims need

The engineer's direction, 2026-11-03, after sabotage controls timed out on runners that stalled under suites doing real file I/O: reduce each heavy test to the size its claim needs, keep its full scale in a long-haul tier that runs daily when main has moved and outside the sabotage sweep, and put the tests that have never run in CI on probation, where failures do not fail the run and results are gathered toward promotion. Recorded as a decision with its working positions (the promotion threshold, where results are kept), the measurements in the rationale, the work as M45, and the sweep-timeout question parked behind it as M45+.1.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…t its tier

Adds tools/check-test-tiers.ps1, which asks every test executable for its list and refuses a test whose ignore reason and module disagree: marked but not in the module, in the module but not marked, a path that contains the filter without a module of that name, a marker with no cost, and a misspelt marker. It runs in the all-features build + test job after cargo test.

libtest writes an ignore reason into its JSON without escaping a backslash, so the administrative-share tests' reason is not valid JSON; the census reads such a line by its shape.

tools/test-check-test-tiers.ps1 tests each rule in both directions, from recorded listings and against a fixture workspace, on PowerShell 7 and Windows PowerShell 5.1, and tools/sabotage-tiers.json re-injects each rule's defect (15 entries, all behaving as declared). DEVELOPMENT.md and the repository instructions state the convention.

Completed item: M45.1: The tiers' convention and its census. A long-haul test is #[ignore = "long-haul: what it costs"] and sits in a module named long_haul; a probation test is #[ignore = "probation: what it costs"] in a module named probation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… whether main has moved

Adds .github/workflows/long-haul.yml: a plan job and a tiers job. The tiers job runs the long-haul tier and then the probation tier with --ignored and the tier's module filter, over the library, binary, integration and example test targets. A failure in the long-haul tier fails the run; the probation step is continue-on-error and its outcome goes in the run summary.

tools/plan-long-haul.ps1 decides whether the schedule has anything to do: a dispatch always runs, and a schedule runs unless the most recent earlier run that completed was on the commit being run now. A cancelled run did not complete. When the earlier runs cannot be listed it runs, with a warning.

tools/test-plan-long-haul.ps1 tests each rule in both directions on PowerShell 7 and Windows PowerShell 5.1, including the gh path through a fake gh and the arguments it receives, and runs in CI with the other harness suites. tools/sabotage-long-haul.json re-injects each rule's defect (16 entries, one a declared equivalent mutant). Windows PowerShell 5.1 returns a parsed JSON array as one object, which the planner now enumerates.

The workflow has not run on a runner; the commands were run on the workspace and on a fixture crate.

Completed item: M45.2: long-haul.yml: daily, and only when main has moved since the last completed run, plus manual dispatch. It runs the long-haul tier across the workspace and a failure there fails the run, then the probation tier, whose failures do not.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ch and report them

Adds tools/run-tier.ps1, which owns the command that selects a tier and reads each test's outcome from libtest's JSON events, keyed by the source path of the target its executable was built from. A test that started and never reported (its executable aborted) is a failure. long-haul.yml now runs both tiers through it.

Adds tools/publish-probation-results.ps1, which applies a run to results.json on a probation-results branch that shares no history with main: once per run id, as a fast-forward, never forced, retried from a fresh fetch. A record job holding the only write access publishes it from the artifact the read-only tests job uploads, and tools/report-probation.ps1 prints each test's runs, passes, current run of passes and latest result, with a candidate threshold that is a working position.

tools/test-probation.ps1 tests the parse, the record, the report, the publisher against a bare remote and the runner against real workspaces on PowerShell 7 and Windows PowerShell 5.1, and runs in CI. tools/sabotage-probation.json re-injects 31 defects and a control, all behaving as declared. The record keeps tests in an ordinal dictionary and a list, since PowerShell's ordered dictionaries and object properties compare keys without regard to case.

The workflow has not run on a runner. The publisher is a copy of the sabotage record's on purpose; merging them is parked as M45+.2.

Completed item: M45.3: Keep each probation test's results across runs, and report them. Where the results live is decided and the promotion threshold stays a working position until the first results exist.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…t a time

Moves windows-ioring-sys' five flush-barrier stress instruments and win-durable-ioring's 4 GiB buffer test into a probation module with probation: ignore reasons, so the probation tier selects them and their results are kept across runs. The census reports six probation tests. Their names are unchanged; tools/soak-flush-barrier.ps1, which runs them with --exact, now passes the module path.

Putting them on probation showed that tools/run-tier.ps1 ran a tier's tests in parallel, which the stress instruments' own docs say is not optional decoration: three start contention writers, and two once collided on a temp file under a parallel harness. run-tier.ps1 now passes --test-threads, one by default, with -TestThreads to raise it. A test with an overlap-detecting fixture covers both directions and a sabotage entry removes the argument; the five affected entries behave as declared. The choice is recorded as one made here.

All six passed on their first run, through run-tier.ps1 for the two crates with IORING_STRESS_TRIALS=5. The default trial budget was not run.

Completed item: M45.4: Put the never-run tests on probation: windows-ioring-sys' five flush-barrier stress tests and win-durable-ioring's test that allocates a buffer of more than 4 GiB.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…st at the size its claim needs

The example now takes the instance's declared operations as a parameter (run keeps the demonstration's 256) and seals an epoch every eighth of them. The test that writes past the declared capacity does it at 32, with 96 writes under each delivery, and its full-scale form, three times 256 under each, is a long-haul test. The tests that wrote 200 writes through a queue of 4 write 40. The example's tests take about a tenth of a second.

The smaller size lost a detection that the sabotage sweep showed: at 16 operations an epoch is smaller than the queue, so a pop is never refused for want of room and the entry for that survived. It was caught at 24 and 32, and 32 is kept. All 9 entries that patch the example behave as declared, and the two find strings that named EPOCH_WRITES follow it to epoch_writes.

Completed item: M45.5: win-durable-ioring's blocking_push example: write past a small declared capacity, not three times 256 writes, and keep the full scale as a long-haul test.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ir claims need

The three tests that built a directory of 4000 files (thousands of entries delivered exactly once, a cancelled large running enumeration, a dropped receiver on one) need many refills of the native buffer and several quanta, not thousands of files; creating the files was what each cost. Each now takes a count and a buffer size and runs over 600 files with the minimum buffer, and keeps its original form, 4000 files through the default buffer, as a long-haul test under its original name. The integration suite takes under a second, and the three long-haul forms pass through the long-haul tier.

All 28 entries of the crate's sabotage manifest, swept whole because the tests changed and not the code they judge, behave as declared.

Completed item: M45.6: windows-file-enumeration-sys: the directory of thousands of entries and the two large running enumerations that are cancelled or dropped. Each to the size its claim needs, the full size long-haul.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ims need

Seven tests of win-multi-file-tail-watcher took their size from how large the thing was first written, not from what they assert. Each now takes its size as a parameter, runs small, and keeps its original size as a long-haul test beside it, and all seven long-haul forms pass through the long-haul tier.

The two roster tests assert a ratio, a step against a pass, and skip the comparison under a 10 ms pass, so they need a measurable pass and not a large one: 5000 files gives about twice the floor on the machine measured on, and 2000 gave less than the floor, which would have made the claim vacuous. The job logs write a tenth of the steps, the two child-output tests use 24 child processes where they used 200 and 150, and the park-and-resume test registers 100 files where it registered 300, with the constants it asserted against derived from the writer count.

The quiet-between-bursts waits are left and the azure-shape test says why: they are claims about time.

The 20 sabotage entries the published catcher record says these tests caught all behave as declared.

Completed item: M45.7: win-multi-file-tail-watcher: the large roster dropped during a poll, the job log under the tiny and default options, the children whose output is reassembled, and the real-writer tests. Shrink where the scale is not the claim; where it is a timing claim, say so and leave it.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…d run their tests at it

The cost probes took only a whole-number multiplier of at least one, so a test of what a probe says, and not of its figures, could not run it lighter. They now take --iteration-divisor N, the counterpart of --iteration-multiplier: each loop runs a Nth of its usual count and at least once, so a shortened run goes round every loop and its report has every line a full one has. A run is lengthened or shortened, never both. A shortened run says so beside its banner, and every row now carries iteration_divisor, 1 by default. IterationMultiplier::FLAGS is now two flags and the type gains divisor().

The census, the table-matches-row tests and the multiplier test run the cost probes short: the request probe's run went from about 5.5 s to 0.3 s and the census from about 12 s to about 6 s. Each keeps its full-length form as a long-haul test, and all four pass through the long-haul tier. The peer-index-cache and core-affinity probes measure through windows-placement-probe, whose workload is a public constant, so they still run in full; that needs a decision about a second crate's public signature and is a checklist item.

Nine sabotage entries cover the divisor. The 16 entries whose patched file changed and the 15 the published catcher record names as caught by a test this changed all behave as declared.

Completed item: M45.8: windows-platform-probes: the tests that run every registered probe, or a cost probe at a multiplier, at full workload. Run the probes at the smallest workload that still shows what each test checks, with full workload long-haul.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The engineer ruled that the six seconds the census still takes, most of it the peer-index-cache and core-affinity probes, is not a problem, so a setting for their size, which would change the public API of a second published crate, is not worth building. The item is removed from the checklist and archived as withdrawn with the ruling quoted, and the M45.8 entry that called it open now says it was withdrawn.

Completed item: M45.8.1: windows-placement-probe: a setting for how many items its peer-index-cache and core-affinity measurements move. Withdrawn, not done.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
M45.9 asks for a bound to be shortened only where the claim holds at the shorter one. These four do not, with the margin that keeps them from flaking on a stalled runner, so each is now a named constant with the measurement beside it and is otherwise unchanged. win-job-launcher's grandchild and orphan tests: the descendant holds the marker after about 60 ms and 160 ms when idle and about 0.35 s and 1.0 s with every processor busy, so the 3 s bound is three times the loaded figure. win-debugging-sys: a shorter bound would still pass, for a process ended before it started, but without showing it was running. win-durable-ioring: the 600 ms stall against a 50 ms bound is what tells a bound that is honoured from one that is ignored, and shortening it would not shorten a suite whose longest test is about as long.

No value changed, so no sabotage sweep was run.

Completed item: M45.9: The tests whose wait is their claim: win-job-launcher's bound killing a descendant or a grandchild, win-debugging-sys' process that outlives its bound, and the registration bounds in win-durable-ioring. Shorten a bound only where the claim holds at the shorter one; otherwise leave it and say why.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
windows-file-watcher's stress.rs and scenario_stress.rs returned at once unless WINDOWS_FILE_WATCHER_STRESS was set. Nothing in CI sets it, so all 17 reported as passing in 0.00s while running nothing, and the tier census could not see them because none was ignored. The guards are gone; each test is in a probation module with a probation ignore reason carrying its measured cost. Run through tools/run-tier.ps1 they take about 237s serially and all pass.

The workspace was re-measured serially: 140s to 101.6s, and tests of a second or more from 22 (65s) to 13 (31.5s). Each of the 13 is left with a reason recorded in the archive. The two parked M45+ items get gates that name what they now wait for.

Completed item: M45.10: Review the remaining tests of a second or more against the same rule, starting with windows-file-watcher's scenario stress and windows-ioring-sys' properties under every resolution, by re-measuring the workspace once M45.5 to M45.9 are in.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings November 4, 2026 20:08

Copilot AI 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.

🟡 Changes recommended

Unresolved issues can publish incomplete captures, double-count probation passes when recording is rerun, and break tier execution when Cargo arguments contain spaces.

3 open issues
What changed in this PR

This PR separates expensive and probationary tests from routine CI, reducing push-time workloads while preserving full-scale coverage in scheduled runs.

Changes:

  • Adds tier conventions, validation, scheduled execution, and persistent probation reporting.
  • Reduces scale-heavy tests and adds full-scale long-haul variants, while retaining waits required by timing claims.
  • Adds cost-probe iteration divisors and sabotage coverage for the new tooling.
File Description
tools/​soak-flush-barrier.ps1 Uses probation-qualified stress-test names.
tools/​sabotage-tiers.json Defines mutations for tier validation.
tools/​sabotage-tiers-runner.cmd Adapts tier-validation tests to sabotage runs.
tools/​sabotage-probation-runner.cmd Adapts probation-tooling tests to sabotage runs.
tools/​sabotage-long-haul.json Defines mutations for scheduling decisions.
tools/​sabotage-long-haul-runner.cmd Adapts planner tests to sabotage runs.
tools/​run-tier.ps1 Runs a selected tier and records outcomes.
tools/​report-probation.ps1 Reports probation history and promotion candidates.
tools/​README-sabotage.md Documents the new tooling sabotage suites.
tools/​plan-long-haul.ps1 Gates scheduled runs on changes to main.
PARKED.md Tracks timeout and publisher follow-up decisions.
DEVELOPMENT.md Documents tier conventions and usage.
DESIGN-RATIONALE.md Records tier motivation and reporting trade-offs.
DESIGN-NOTES.md Defines tier execution and probation-record contracts.
crates/​windows-platform-probes/​tests/​the_probes_that_emit_a_machine_readable_row.rs Shortens routine census runs; adds long-haul coverage.
crates/​windows-platform-probes/​tests/​a_probe_takes_its_settings_or_refuses_them.rs Tests divisor settings and tiers expensive runs.
crates/​windows-platform-probes/​tests/​a_cost_probe_tells_both_readers_the_same_thing.rs Checks shortened reports and preserves full-length variants.
crates/​windows-platform-probes/​src/​probe_args/​tests.rs Tests divisor parsing, scaling, and reporting.
crates/​windows-platform-probes/​src/​probe_args.rs Adds iteration-divisor support.
crates/​windows-platform-probes/​src/​bin/​request_cost.rs Includes the divisor in machine-readable output.
crates/​windows-platform-probes/​src/​bin/​doorbell_cost.rs Includes the divisor in machine-readable output.
crates/​windows-platform-probes/​sabotage.json Updates patches and adds divisor mutations.
crates/​windows-platform-probes/​DESIGN-NOTES.md Documents shortened probe runs.
crates/​windows-file-enumeration-sys/​tests/​integration/​scale.rs Adds smaller enumeration coverage and a full-scale tier.
crates/​windows-file-enumeration-sys/​tests/​integration/​cancellation.rs Reduces routine directory sizes; preserves full-scale tests.
crates/​win-multi-file-tail-watcher/​tests/​shutdown_every_state.rs Uses smaller rosters with long-haul counterparts.
crates/​win-multi-file-tail-watcher/​tests/​reader_real_writers/​runner_log.rs Shortens bursts and preserves full-length log tests.
crates/​win-multi-file-tail-watcher/​tests/​reader_real_writers/​azure_shape.rs Explains why silence waits remain unchanged.
crates/​win-multi-file-tail-watcher/​tests/​poll_real_writers.rs Reduces routine writer counts; adds long-haul coverage.
crates/​win-multi-file-tail-watcher/​src/​read_layer/​child_tests.rs Reduces child counts and retains the larger test.
crates/​win-multi-file-tail-watcher/​src/​read_engine/​child_tests.rs Reduces child counts and retains the larger test.
crates/​win-job-launcher/​tests/​launch.rs Names and explains the unchanged descendant timeout.
crates/​win-durable-ioring/​src/​windows_backend/​tests/​construction.rs Names and explains the unchanged registration stall.
crates/​win-durable-ioring/​src/​windows_backend/​tests.rs Moves the large-buffer test into probation.
crates/​win-durable-ioring/​src/​dioring/​tests/​construction.rs Names the unchanged construction-test stall.
crates/​win-durable-ioring/​sabotage.json Updates the epoch-sealing mutation target.
crates/​win-durable-ioring/​examples/​blocking_push/​tests.rs Tests smaller capacity and preserves full-scale coverage.
crates/​win-durable-ioring/​examples/​blocking_push/​main.rs Parameterizes capacity and epoch sizing.
crates/​win-debugging-sys/​tests/​debugger.rs Names and explains the unchanged debugger timeout.
CHECKLIST.md Records M45 work and completion links.
.github/​workflows/​long-haul.yml Schedules tier runs and publishes probation results.
.github/​workflows/​ci.yml Adds tooling tests and tier validation.
.github/​copilot-instructions.md Adds contributor guidance for test tiers.

Review effort: Balanced

Comment thread .github/workflows/long-haul.yml Resolved
Comment thread tools/run-tier.ps1 Resolved
Comment thread tools/run-tier.ps1 Resolved
Mike Grier and others added 3 commits November 4, 2026 18:49
…guments whole, refuse a capture it could not read

The sabotage harness tests failed on the runner: cargo there writes its Running headers with colour escape codes (CARGO_TERM_COLOR=always reproduces it locally), so no executable was recognised and three run-tier cases could not read their capture. The run now passes --color never.

Two review findings on the same script. The run's arguments were joined with spaces into a batch file, so a path with a space came apart; each is now quoted for cmd, percent signs are doubled, and an argument with a double quote is refused. And an event line that could not be read was only logged while a partial outcomes.json was written and the step could exit 0; the step now exits 2 and writes none, since the record reads an unreported test as one that was not run.

One test case for each, and five sabotage entries, all caught.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The record job can be re-run alone over the same outcomes artifact. It named the run by its own github.run_attempt, so the re-run published the same outcomes under a new id and Add-ProbationRun counted passes and streaks again for tests that did not run again. The job that ran the tests now writes its run and attempt beside outcomes.json in the artifact, and the record job publishes under that.

Nothing exercises the workflow text, so this one has no test; the first scheduled run is what shows it works, and a re-run of the record job alone after it is what shows the id holds.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings November 5, 2026 00:09

Copilot AI 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.

🟡 Changes recommended

Push-time CI still selects the full-length long_haul probe tests on every PR and push instead of reserving them for the daily workflow, so its invocation needs the long_haul and probation skip filters.

1 open issue
3 resolved since last review

Review effort: Balanced

Mike Grier and others added 2 commits November 4, 2026 19:54
…-tier step, and check every workflow for the same

The probes job ran cargo test -p windows-platform-probes with --include-ignored, which selects every ignored test not skipped, so the full-length probe tests M45.8 moved into long_haul would have run on every push. It now skips both tiers.

check-test-tiers.ps1 now reads the workflows' cargo test commands that run ignored tests (--include-ignored or --ignored) and refuses one that would select a tier test, working out the selection from the census: its packages, its targets, its name filters and its skips. The census records each executable's package and kind for it. Run over the repository as it stood, it reports this one command and no other. Seven test cases, both directions, and seven sabotage entries, all caught.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… hardware-dependent change found

The sweep of main after #177 merged had a control hang past its bound inside the blocking_push capacity test that M45.5 made smaller. It is a data point for M45+.1, from before M45 landed; whether a stall still reaches a control after it is what the next sweeps show.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings November 5, 2026 00:54
@MikeGrier

Copy link
Copy Markdown
Owner Author

Round summary, after 98d2e0b:

  • Review thread (probes --include-ignored): fixed in 8f2a74a, with a check over every workflow so the class cannot recur; replied on the thread.
  • The failing sabotage sweeps run on main (after Let a sabotage entry declare its result hardware-dependent #177 merged) is not from this PR: shard durable-ioring 25/49 had the control "the abandoned-through bound takes its operands in the other order" hang past its 21 s bound, with every real entry caught. The test it was stuck in is blocking_push's capacity test, which main still has at full size and which M45.5 makes smaller. It is recorded as a data point under M45+.1 in PARKED.md. Nothing here changes the harness's handling of a timed-out control; that is the parked question, and one hang before M45 landed does not answer it.
  • Earlier this afternoon: the sabotage harness tests failure (cargo colour escapes in the capture) was fixed in 994c933.

Copilot AI 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.

🔵 Needs a closer look

Directory paths containing literal percent signs or parentheses remain mishandled, and A/B/A artifact replays can double-count runs and overwrite newer results, potentially hiding failures.

No open issues 🎉

3 previously missed issues

In code that hasn't changed since last review

Medium severity Prevent replayed runs from replacing newer probation results

.github/​workflows/​long-haul.yml:157

After runs A and B publish, replaying A's record job with its retained artifact counts A again and replaces B's newer results, potentially hiding a failure. Publish-Once and Add-ProbationRun compare only against latestRun, so preserving the producing ID prevents consecutive replays but not this A/B/A case. Track applied producing IDs and preserve result chronology in the shared record and publisher logic, with an A/B/A regression test.

Medium severity Escape literal percent signs in output-directory paths

tools/​run-tier.ps1:110

An OutputDirectory containing literal %USERNAME% is created as written, but cmd expands that token in the unescaped capture redirection, writing elsewhere or failing before cargo starts. Preserve literal percent signs in both the redirection and the generated batch-file invocation path. Add an output-directory regression case; the existing --target-dir case does not cover these paths.

Medium severity Accept parentheses in executable paths when parsing capture headers

tools/​run-tier.ps1:125

A valid --target-dir containing parentheses still fails at this reader even though it now reaches cargo correctly. ConvertFrom-ProbationCapture recognizes Running headers using (?<exe>[^()]+), so a header naming C:\tier (debug)\debug\deps\fixture-abc.exe is ignored; the first test event then throws because no executable was named. Update the shared parser to accept parentheses within the executable path and add a target-directory regression case.

1 resolved since last review

Review effort: Balanced

… and read paths with percent signs and parentheses

Three review findings on the probation tier's tools.

A run replayed after a later one (a re-run of the keeping job over an older artifact) was counted again and its results put over the later run's, because only the latest run was compared. The record now keeps every run applied, in order, and the publisher and Add-ProbationRun refuse any of them; a record written without the list counts its latest run as applied. Tests: the in-memory replay, a round trip through the record text, and the same replay through the publisher.

An output directory with a percent sign in its name had cmd expand the name in the capture redirect and in the path of the generated batch file. The redirect doubles the percent signs, and the batch file is written to the temp directory so its path is not the caller's.

An executable path with parentheses above it, as in Program Files (x86), was not recognised in a Running header. The executable is now read by its file name from the end of the line.

Seven sabotage entries, and the two older ones that named the replaced comparison repointed; all 43 behave as declared.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings November 5, 2026 02:00
…latest

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@MikeGrier

Copy link
Copy Markdown
Owner Author

Review 5778261998 raised three issues, none as inline threads, so one reply for the round. All three were valid; fixed in a6c1423.

  • A/B/A replay of the record job. Comparing only latestRun let a replay of an earlier run count again and put its results over the later run's. The record now keeps every applied run id (one a day), and both Add-ProbationRun and the publisher refuse any of them; a record written without the list counts its latest run as applied. Tests: the in-memory replay, a round trip through the record text, and the same replay through the publisher against a real remote.
  • Percent signs in OutputDirectory. The capture redirect now doubles %, and the generated batch file is written to the temp directory so cmd never reads a %NAME% in the path it is started with. Test: an output directory named out %USERNAME% dir is used as named and the expanded directory is never created. My earlier test covered only --target-dir, as the review said.
  • Parentheses in executable paths. The Running header is now read by the executable's file name from the end of the line, which cargo makes from a hash and which has no parenthesis in it. Test: a header under C:\Program Files (x86)\work (2)\....

Seven new sabotage entries and two older ones repointed at the changed comparison; the sweep of the probation manifest (43 entries) behaves as declared. The suite also passes under Windows PowerShell 5.1.

The common thread of the last two rounds: the first fix for each finding covered the case the reviewer named and not the neighbouring one (a --target-dir with % but not an output directory with %; the latest run but not an earlier one).

Copilot AI 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.

🔵 Needs a closer look

Human assessment is needed because case-insensitive tracking of concurrent tests can omit a crashed test and leave its previous streak unchanged.

No open issues 🎉

Review effort: Balanced

… that breaks a workflow's YAML

8f2a74a added --skip long_haul:: --skip probation:: to an unquoted run value, and a plain YAML scalar cannot hold a colon and a space. GitHub did not run the ci workflow at all for the two pushes since: the run failed with no jobs, so the pull request had no build + test check and could not merge, with nothing saying why beyond a skipped release job. The value is now quoted.

Nothing read the workflows as YAML, so a new test in the release automation's suite (which runs on every push) parses each one and reports its errors. It also pins both directions on a snippet: the unquoted command is an error and the quoted one is not. Against the version of ci.yml at f04f1f4 it reports the line and column.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings November 5, 2026 02:26

Copilot AI 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.

🔵 Needs a closer look

The new Windows Actions workflow and write-enabled recording job still require end-to-end runner validation of schedule gating, artifact handoff, record-only reruns, and authenticated results-branch publication.

No open issues 🎉

Review effort: Balanced

@MikeGrier
MikeGrier merged commit 3f914cf into main Nov 5, 2026
47 checks passed
@MikeGrier
MikeGrier deleted the mikegrier/long-haul-tier branch November 5, 2026 02:34
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.

2 participants