Skip to content

Fix flaky credential-refresher liveness assertions in lib/test/run.sh (widen slack budgets) #1931

Description

@prflow-implementer

Dependencies

Blocked by #1923 — this follow-up characterizes and fixes assertions the parent issue's PR left deferred, so the parent's implementation must land first.

Problem Statement

Some test-suite checks fail only sometimes, and only when the machine running them is busy. Three checks like this live in the test suite: they watch for a credential refresher to stay alive, and they turn red only after the suite has already been run several times in a row on the same machine. Fixing that costs real time: two recent runs together spent nearly thirteen minutes just trying to work out whether these three checks were broken or just unlucky, and both times the answer was "unlucky machine timing," not a real bug.

Current Behavior

The environment fact is that this failure depends on the environment: the three checks pass on a quiet machine and fail only under sandbox contention (heavy concurrent load), which is the defect.

  • The credential-refresher liveness checks named #487 arm13, #491 arm20b, and #1882 arm1882k, all living in lib/test/run.sh, go red only after the whole test suite has been run repeatedly, back to back, in one sandbox.
  • The project's own rule says a check that fails only under load is never waved through as "probably fine" — it must be treated as a real failure until proven otherwise.
  • The project's own rule also already names the fix: when a check like this fails only under load, the fix is to give that specific check more slack (a wider timing allowance), not to loosen the overall no-flake rule.

Desired Behavior

Each of the three named checks passes reliably even when the whole suite is run repeatedly, back to back, in one sandbox. Each check reaches that reliability by widening its own timing allowance, not by weakening any project-wide rule about how failures are treated. The project-wide rule that forbids waving off a load-sensitive failure, and the rule that says to fix the check's own slack instead, both stay in place, worded no more weakly than before.

User Impact

A developer or an automated run no longer burns minutes rerunning the whole suite to figure out whether one of these three checks is a real failure or just sandbox noise. The check tells the truth on the first run.

Technical Context

Scope note: The files and details below are the known starting points, not the full list. Before implementing, trace the change through the codebase to find every affected call site, consumer, and layer — this issue maps the work, it does not bound it.

  • Relevant Classes/Files — lib/test/run.sh (the three named assertions and their credential-refresher fixtures), CLAUDE.md (the no-known-flake rule and its slack-budget remedy).
  • Architecture Alignment — this is a change to the timing allowance inside three existing assertions, not a new mechanism; the assertions' surrounding fixtures and harness stay as they are.
  • Cross-layer Impact — the shell test suite only; no other layer changes.

Acceptance Criteria

Implementation Notes

  • Approach — run the three named assertions repeatedly, back to back, in one sandbox to characterize how much timing slack each one needs under contention, then widen each assertion's own allowance by that amount, leaving the behavior it checks and the surrounding harness unchanged.
  • Relevant files — lib/test/run.sh and its credential-refresher fixtures.
  • Code Patterns — follow the existing shape of the three named assertions; widen their timing/retry allowance in place rather than introducing a new mechanism.
  • Testing Strategy — run the three assertions repeatedly in one sandbox under contention and confirm they stay green; for each, also confirm it still goes red against a mutated copy of the behavior it pins, so the widened slack has not disabled the check.
  • Documentation Needed — none. CLAUDE.md's existing rule and remedy text are not changed by this work; they are preserved as they are.
  • Potential Gotchas — do not fix this by adding a known-flake exemption or by loosening CLAUDE.md's no-known-flake rule; the fix is scoped to the three assertions' own slack budgets. A separate credential-refresher investigation may exist elsewhere — check for one before assuming this work is independent of it.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    DeferredCreated by DevFlow automationPRFlow

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions