import_picker real-terminal smoke: positional command to script(1) fails on util-linux (Linux gate red on master) #61

Activity

1. ckcred-alfonso commented on Sep 19, 2026

   

   ckcred-alfonsoBot

   on Sep 19, 2026

   More actions

   Confirmed at source, and your mechanism is exactly right — but the title needs one correction that changes who is affected.

   Master's CI is green, and always was. The arm lives in scripts/gate.sh (line ~471, run_expect 8 "interactive import picker + real-terminal smoke") and appears zero times in .github/workflows/ci.yml. Last four master runs: 456a34a, bdd2c25, 25971eb, 7080773, all success on both legs. So what is red is the LOCAL gate on any util-linux host — which is worse in one way than a red CI, because it means this PTY coverage has only ever run on a maintainer's laptop and a regression in it would reach nobody. That second gap is mine, not yours, and it is being closed in the same change.

   Taking your option 2, for the reason you gave plus one more. libc is already a dev-dependency of credentials-module — it is what supplies the deadline SIGKILL in that same helper — so openpty costs no new dependency and collapses the two platform behaviours into one code path. The extra reason against option 1: util-linux script -c passes its command through a shell, so the fixture's temp paths become shell-interpolated and a quoting bug becomes possible on a path where none exists today. Passing argv straight to the child avoids inventing that surface.

   Option 3 I would not take for the reason you already named: skipping on Linux loses the coverage on the platform the vault actually runs on in most deployments.

   What I cannot do and you can. I can only exercise this on macOS. The change is one code path for both platforms, so it either works everywhere or fails everywhere — but 'I believe it works on util-linux' is not a measurement. When it lands I will ask you to run cargo test --locked -p credentials-module --test import_picker -- --test-threads=1 on your 2.42.2 host before I close this. Your reproduction (script -q /dev/null /bin/echo --data-dir versus the -c form) is the kind of thing that makes a report actionable without a round trip, and the same applies in reverse here.

   One thing worth stating since you flagged it as unchecked: you were right to check rather than assume the ubuntu job runs the picker arm. It does not, and that absence is the finding underneath your finding.
2. iceteaSA commented on Sep 19, 2026

   

   iceteaSA

   on Sep 19, 2026

   ContributorAuthor

   More actions

   Agreed on all three: the arm is local-gate-only (I had not checked ci.yml), option 2 via libc::openpty is the one that removes the platform fork rather than papering it, and the -c form's shell interpolation of fixture paths is a surface that does not exist today — not worth creating.

   Standing offer accepted: ping me when it lands and I will run

   cargo test --locked -p credentials-module --test import_picker -- --test-threads=1
   

   on this host (script from util-linux 2.42.2, kernel Linux) and paste the result here. I will run it on a quiet box and with the sibling pins matching the commit's Cargo.lock — the manifest-lock timing tests in the same crate have taught me not to report a red from a loaded machine.
3. ckcred-alfonso commented on Sep 19, 2026

   

   ckcred-alfonsoBot

   on Sep 19, 2026

   More actions

   Landed. 9d36cf8 on master, CI green on ubuntu and windows; master is now at f72e250 with two unrelated commits on top.

   What changed, against what you reviewed:

   • script(1) is gone. The harness drives libc::openpty directly, so there is no external tool and no BSD-versus-util-linux fork to get wrong.
   • The CI step you found missing is there now, ubuntu-only. I dropped the macOS leg I had originally asked for: it is already covered by the local gate, which runs on a Mac before every train, and a third runner costs every push. Linux was the platform with no coverage at all, which is exactly the hole your report opened.
   • TIOCSCTTY is kept and documented at the site as undefended. My acceptance criterion had demanded that removing it redden a test; it does not, and the reason is structural rather than a missing assertion — the picker gates on is_terminal(), which is isatty(fd) and observes a tty descriptor, not a controlling terminal. No test can discriminate that call through that predicate. The control that does discriminate is withholding the input bytes, which reddens by name and exercises the deadline.

   Your caveats are the right ones and one of them found a real bug on my side. While gating this I hit quarantine_past_ttl_but_inside_margin_is_retained failing under load, and it was not flakiness in the machine-is-busy sense: three reclaim tests seeded a directory mtime from one clock reading and judged it against a second, so the age under test was "101ms plus however long the test body took". That fixture's entire budget is the reclaim margin minus one millisecond. I fixed it by deleting the helper that permitted the double read rather than documenting it, so every call site must now hold one instant and use it for both the seed and the verdict.

   So if you see a red in opencode_files.rs on a loaded box, it is worth a second look at whether master is current — but import_picker itself should be timing-independent.

   The invocation you named is the right one.
4. iceteaSA commented on Sep 19, 2026

   

   iceteaSA

   on Sep 19, 2026

   ContributorAuthor

   More actions

   Green on util-linux. Master 3d862a1 (carries 9d36cf8), fresh detached worktree, siblings pinned to the lock (subc-core 0.18.1, subc-protocol 0.21.0, subc-transport 0.7.0, cortexkit-store-types 0.2.2; cargo metadata --locked --offline resolves), load 2.5, script from util-linux 2.42.2 (irrelevant now, recorded for the record), kernel 7.2.5.

   $ export CK_AUTH_IMPORT_SHIPPED_BINARY="$PWD/target/release/ck-auth"
   $ cargo test --locked --offline -p credentials-module --features import-prompt-seam --test import_picker -- --test-threads=1
   test result: ok. 8 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.97s
   test result: ok. 8 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.98s   (second run)
   

   real_terminal_adapter_exercises_submit_controls_drop_and_attempt_bound passed both runs.

   One correction to the invocation you quoted, worth noting for the next person who runs it from the thread: without --features import-prompt-seam the test binary compiles with zero tests and reports test result: ok. 0 passed; 0 failed; ... finished in 0.00s — the exact void-green shape from the gate.sh skip-notice discussion. My first run did that and I nearly pasted it here as a pass. The gate's own arm (scripts/gate.sh:478-480) carries the feature and the CK_AUTH_IMPORT_SHIPPED_BINARY export; the numbers above are from that form.

   Noted on the reclaim fixture fix (one instant for seed and verdict) — that is the same clock-split shape as the resolve_now_ms/current_time_ms split I fixed in PR #33's release path, so I would expect the load flakes in opencode_files.rs to be gone rather than rarer. Closing is yours.
5. ckcred-alfonso commented on Sep 19, 2026

   

   ckcred-alfonsoBot

   on Sep 19, 2026

   More actions

   Closing. Green on util-linux, both runs, and your correction to my own invocation is the more useful half of that comment.

   My quoted command was void-green-capable and I published it. Without --features import-prompt-seam the binary compiles with zero tests and reports ok. 0 passed, which is indistinguishable from a pass. I wrote that invocation in this thread as the thing to run; you nearly pasted its output back as a verification, and the only reason you did not is that you looked at the count. The gate arm carries the feature and the CK_AUTH_IMPORT_SHIPPED_BINARY export; the comment did not, and the comment is what the next person copies. Corrected form, for the record:

   CK_AUTH_IMPORT_SHIPPED_BINARY="$PWD/target/release/ck-auth" \
     cargo test --locked -p credentials-module --features import-prompt-seam \
     --test import_picker -- --test-threads=1
   

   That is the third instance of this shape I have hit today — a name filter matching nothing, a mutation anchor matching one of two sites, and now a feature-gated target compiling empty. All three report success in the same words as a real pass.

   One correction on your last paragraph, because otherwise you will read a recurrence as a regression of my fix. The opencode_files.rs load flakes are not gone. I hit them twice more after the reclaim fix, at load 111 on a shared machine, and the reclaim fixtures were not the cause — that fix holds, each of those tests now takes one instant and uses it for both the seed and the verdict.

   What I found instead was a separate defect in the production code: ManifestLease::commit had three distinct refusal paths sharing one message — the renewal thread died, the owner file stopped naming us, or the claim aged past its TTL. So commit_measures_the_lease_clock_not_the_wall_clock failed with a message saying the clock had been read, while passing 20 of 20 in isolation, and sent me back to the clock fix I had already landed. The condition was known one line above each refusal and thrown away. Each arm now names itself, including a distinct expired against its own clock the test can key on, and the test no longer asserts a cause it has not checked.

   The NotFound-on-a-just-created-directory shape is still open with no mechanism (#51). It now re-stats the path and lists the parent instead of reporting a bare Os { code: 2 }, deliberately without a retry — a retry would make it disappear and take the evidence with it, and I do not yet know which failure I would be papering over.

   Both of those are on an integration branch and will reach master with the enrollment work.

   Thanks for running it on a quiet box with the siblings pinned, and for reporting the load. A green from a loaded machine and a green from a quiet one are different claims, and only one of them was worth having here.
6. ckcred-alfonso

   closed this as completedon Sep 19, 2026