Repository navigation
Start the test only once the unlocked keyring owns the Secret Service - #118
Conversation
Display mode started each run under xvfb-run --auto-servernum, whose check-then-start can give two jobs on one machine the same display. Each run now starts its own Xvfb with -displayfd, as the build and test's end-to-end job does, opens it only to a new xauth cookie, waits at most 30 seconds for the display's number, and stops Xvfb after the run. Agent: DEV1 (developer) Agent-Model: Anthropic Opus 5.5, Medium
gnome-keyring-daemon --unlock returns before the daemon takes org.freedesktop.secrets on the session bus, so a test that asks for a secret at once can make D-Bus start a second keyring daemon, with no unlocked login keyring. The end-to-end job and the flake proof's display mode now wait with gdbus wait, which doesn't start the service, for at most 30 seconds, and fail with an error when the name has no owner by then. Agent: DEV1 (developer) Agent-Model: Anthropic Opus 5.5, Medium
…he cookie file first Stopping Xvfb, after a run and in the exit trap, ignores kill's and wait's status, so an Xvfb that has already died doesn't end the step under errexit. The cookie file exists before xauth adds to it, so xauth doesn't print that it doesn't exist. Agent: DEV1 (developer) Agent-Model: Anthropic Opus 5.5, Medium
So xauth doesn't print that the file doesn't exist, as the flake proof already does. Agent: DEV1 (developer) Agent-Model: Anthropic Opus 5.5, Medium
rostislavrotaru
left a comment
There was a problem hiding this comment.
Reviewed: #118 at 59e6828
Verdict: Changes needed
Findings: 2 Should fix, 1 Low, as line comments
Why it's right: Waiting with gdbus wait for the keyring to own org.freedesktop.secrets before the test starts removes the race between the keyring taking the name and the app asking for it, without starting the service itself.
Agent: LEAD (lead)
Agent-Model: Anthropic Opus 5.5, High
Each run may take an even share of the job's time still left, keeping three minutes for the artifact, through GNU coreutils' timeout or gtimeout; a run that reaches it is named with an error and counted as failed, instead of the job's timeout ending the job without naming the run. A runner with neither, such as a stock macOS one, runs without the limit and says so. The README gives the reasons for the per-run limit and for the 30 seconds Xvfb has to name its display. Agent: DEV1 (developer) Agent-Model: Anthropic Opus 5.5, Medium
…he wait's reason Merges dev1/115-flake-proof-display at 0edc485, so this branch holds #116's round-1 and round-2 fixes, the trap's || true and the cookie file's touch among them; the README's display bullet keeps this branch's wait and #116's Xvfb reason. A run whose keyring never takes org.freedesktop.secrets now marks it, and the flake proof ends the job with an error naming the run, as for Xvfb, instead of counting the run as a failed test. The README gives the reason for the 30 seconds gdbus waits: in the trial each run, the wait included, took under half a second. Agent: DEV1 (developer) Agent-Model: Anthropic Opus 5.5, Medium
rostislavrotaru
left a comment
There was a problem hiding this comment.
Reviewed: #118 at 59359b6
Verdict: Approve
Findings: 0 Should fix, 0 Low, as line comments
Why it's right: each run starts only once the keyring owns org.freedesktop.secrets, a keyring that never takes it ends the job naming the run instead of counting a flake, both 30 s limits carry their measured reason, and the branch now holds #116's head. It merges after #116.
Agent: LEAD (lead)
Agent-Model: Anthropic Opus 5.5, High
…nner A runner without GNU timeout, such as a stock macOS one, ran its runs without a limit. Each run now starts in the background with a watchdog that sends TERM at the run's limit and KILL 30 seconds later, and is stopped once the run ends; a run the watchdog ended gets the same error and counts as failed. Agent: DEV1 (developer) Agent-Model: Anthropic Opus 5.5, Medium
Merges dev1/115-flake-proof-display at 03bc6f1. The keyring-fault check stays before the run's result is counted, after the watchdog is stopped. Agent: DEV1 (developer) Agent-Model: Anthropic Opus 5.5, Medium
…kie (#116) ## Summary The flake proof's `display` mode starts each run's virtual display the way the build and test's end-to-end job does, so two jobs on one machine can't get the same display. - `.github/workflows/flake-proof.yml`, the run step: display mode no longer runs each run under `xvfb-run --auto-servernum`, which checks for a free display lock and then starts Xvfb. Each run now: - starts its own Xvfb with `-displayfd`, so Xvfb picks a free display itself, on the same 1920×1080 screen; - opens it only to a new `xauth` cookie, in a file created first, passed to Xvfb as `-auth` and to the run as `XAUTHORITY`, as `xvfb-run` did; - waits at most 30 seconds for the display's number, and otherwise fails the job with "Xvfb didn't start for run <n>"; - runs inside `dbus-run-session` with the unlocked keyring, as before; - stops its Xvfb after the run, and the step stops any Xvfb left when it exits, in both cases ignoring the status of an Xvfb that has already died, so it doesn't end the step. The step checks for Xvfb, `xauth` and `dbus-run-session` as it did for `gnome-keyring-daemon`, failing on a runner without one. - `.github/workflows/flake-proof.yml`, the run step: each run, display mode or not and on every runner, starts in the background with a watchdog in the shell, which sends `TERM` at the run's limit, an even share of the job's time still left after three minutes kept for the artifact, and `KILL` 30 seconds later, and which is stopped once the run ends. A run the watchdog ended is named with `::error::Run <n> reached its limit of <seconds> seconds` and counted as failed, and a share under a minute fails the job at once. - `.github/workflows/flake-proof.yml`, the install step on a GitHub-hosted runner: installs `xvfb`, `xauth`, `dbus-daemon` and `dbus-bin` beside `gnome-keyring`. - `README.md`, the flake proof's `display` input: describes the display as the workflow now starts it, with the reason for the 30 seconds Xvfb has to name its display; the `timeout-minutes` bullet describes the per-run limit and its reason. The code follows the end-to-end job's Xvfb start in `build-and-test.yml` (#113), moved inside the loop so each run gets its own display, as each got its own `xvfb-run` before. ## Linked issue Closes #115 ## Testing - Tests: this repository's PR checks. This repository has no tests of its workflows. - Manual: a trial branch in a private repository, deleted after its run was read, called this workflow at 498fbc1 with `display: true`, `script: test:e2e`, `prepare`, `reports: _build/e2e`, `runs: '4'` and `jobs: '2'` on `linux-x64`, once on self-hosted runners and once on GitHub-hosted runners. Each run's fixture checked that `DISPLAY`'s X socket exists, found the display's cookie with `xauth list` through `XAUTHORITY`, opened an X11 connection with the cookie and one without it, and checked the session bus and the unlocked Secret Service. - In all 8 runs, the X server accepted the connection with the cookie and refused the one without it, and each run's report and log were kept in its job's artifact. - GitHub-hosted: both jobs passed both their runs, in about 30 seconds each. - Self-hosted: the two jobs ran side by side; 3 of their 4 runs passed. The fourth failed its Secret Service check: D-Bus started a second keyring daemon for the test's request, since the run's unlocked daemon didn't serve the Secret Service yet. The keyring start is unchanged by this PR. - Not checked: the per-run limit, 0edc485 and 03bc6f1, ran in no trial, nor on Windows or macOS. - Not checked: review round 1's commit, 15c113e, ran in no trial; #118's trial ran its parent, 498fbc1, with 40 more side-by-side runs. - Not checked: two jobs racing for one display. The trial's side-by-side jobs never shared display numbers, so none could contend; the race is fixed by design, since Xvfb itself claims a free display with `-displayfd` instead of the check-then-start of `xvfb-run --auto-servernum`. Agent: DEV1 (developer) Agent-Model: Anthropic Opus 5.5, Medium
The conflicts in flake-proof.yml and README.md were between #116's squash and its commits already on this branch, whose content is the same, so this branch's side, #116's lines plus #117's, is kept; the diff against main is #117's change alone. Agent: DEV1 (developer) Agent-Model: Anthropic Opus 5.5, Medium
Summary
The end-to-end job and the flake proof's
displaymode start the test only once the unlocked keyring owns the Secret Service on the session bus. It holds #116's head, 03bc6f1, merged in, and targets main so its checks run; until #116 merges, its diff includes #116's. Merge #116 first..github/workflows/build-and-test.yml, the end-to-end test step, and.github/workflows/flake-proof.yml, display mode: aftergnome-keyring-daemon --unlock,gdbus wait --session --timeout 30 org.freedesktop.secretswaits, without starting the service, until the name has an owner; when it has none after 30 seconds, the test doesn't start and the step or run fails with "The unlocked keyring did not take org.freedesktop.secrets on the session bus within 30 seconds.".github/workflows/flake-proof.yml: a run whose wait fails leaves a marker in its keyring folder, and the flake proof then ends the job with an error naming the run, as for Xvfb, instead of counting it as a failed test, since the runner's keyring, not the test, is at fault.gdbusbeside the other programs, and on a GitHub-hosted runner installlibglib2.0-bin, which holds it..github/workflows/build-and-test.yml, the end-to-end test step: creates the cookie file beforexauthadds to it, soxauthdoesn't print that it doesn't exist, as Start the flake proof's display on a display Xvfb picks, behind a cookie #116 does in the flake proof.README.md: the end-to-end bullet and the flake proof'sdisplayinput say how each waits for the Secret Service, and why 30 seconds: in the trial each flake-proof run, the keyring's start and the wait included, took under half a second.Without the wait,
gnome-keyring-daemon --unlockreturns before its daemon ownsorg.freedesktop.secrets, so a test that asks for a secret at once can make D-Bus start a second keyring daemon, with the default data folder and no unlocked login keyring.Linked issue
Closes #117
Testing
display: true, 40 runs in 4 jobs side by side on self-hosted runners, and 4 runs on a GitHub-hosted runner;All 44 runs and all 4 end-to-end jobs passed every check, the unlocked default collection included, and no run's log shows D-Bus starting
org.freedesktop.secretsitself, which Start the flake proof's display on a display Xvfb picks, behind a cookie #116's trial showed in the run that failed. The GitHub-hosted runners' steps foundgdbusafter the install step.Agent: DEV1 (developer)
Agent-Model: Anthropic Opus 5.5, Medium