Skip to content

test(worker): harden the convergence deadline tests and split the revision file - #2298

Merged
freeqaz merged 3 commits into
mainfrom
test-solid-1013
Oct 13, 2026
Merged

freeqaz merged 3 commits into
mainfrom
test-solid-1013

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

A flake sweep of today's CI (no real flakes) found four preventive items:

Change

  • Database-clock deadline test (fix(worker): judge the convergence deadline by the database clock #2281):
    • It now uses two deployments against a 60 s deadline.
    • With the controller clock 5 minutes ahead, a fresh deployment must stay pending.
    • With the clock 5 minutes behind, a deployment admitted 120 s ago by the database clock (the fixture's workAge) must fail on its first pass with CONVERGENCE_DEADLINE_EXCEEDED and retire its bootstrap.
    • Neither result depends on when the first pass runs. Date.now is replaced through context.mock, which restores it when the test ends, even on failure.
  • Outage-at-deadline test (fix(worker): retire an oauth bootstrap last seen running when the deadline pass hits an outage #2283):
    • Its scripted passes must run before the deadline, so the deadline stays real time and goes from 10 to 30 s.
    • Each deployment must also fail at or after the deadline by the database clock.
  • Split: the 34 credential-source cases (withdrawals, and listed sources that lose their grant or readiness) move unchanged into postgres-worker-agent-revision-credential-sources.test.mjs, together with their helpers. The new file is registered in PostgreSQL Application's parallelFiles and expectedTests.
    • The lane still runs 8 files at a time, and the short files fill slots as they free up.
    • Expected time: about 50 s for the new file and about 100 s for the remaining file.
    • The slowest file is still teardown, at about 145 s.
  • SIGTERM headroom: the termination test records a runtime-sigterm measurement for each wrapper and phase: stopped, elapsed time and budget.
    • The reporter allowlists the measurement's 7 case names and its integer times.
    • It appears in each file's measurements in the lane result.
  • Cancelled cleanup: if a file's cleanup fails after the runner's parent shell is gone, the runner reports cleanup status cancelled with issue cleanup-cancelled, not cleanup-failed. The lane still fails, and aggregation treats a cancelled cleanup as failed.

Verification

  • node --test on ci-runner.test.mjs passes locally for:
    • the allowlisted measurements test, which now has a valid runtime-sigterm sample and three rejected ones;
    • a new test that runs the runner under bash and kills the shell from the fake cleanup. With the parent check forced off, this test fails (status: 'failed' instead of cancelled).
  • No database locally (hosted CI is the proof): both revision files load and skip as expected, 50 and 34 tests, the same 84 as before the split.
  • eslint and prettier --check pass on the touched files.
  • Neither docs page grows: controller-worker.md stays at 2,493 words, after a two-word trim in its neighbouring bullet; the other two are far below the limit.
  • An independent review approved the change with nits, which are applied: the controller-ahead first pass gets 45 s, the lane check fails any cleanup that did not pass, and the new runner test is bounded.
  • Pre-fix reasoning:
  • Hosted CI on head 9f2b968 (full run, green):
  • Head 637bd9b: the cancelled-cleanup test now starts the runner under a node parent instead of bash -c, after code scanning flagged the shell command. CI Required is green on this head.

Risks

…ision file

The two convergence deadline tests from #2281 and #2283 ran a real 10 s
deadline, so a runner stall before the first pass could fail them (the
finding 1171 pattern), and the #2281 test replaced Date.now by hand.

- The database-clock deadline test now uses one fresh deployment for the
  controller-ahead case and one admitted two minutes ago (workAge) for the
  behind case, against a 60 s deadline: neither depends on when the first
  pass runs. Date.now is replaced through context.mock, which restores it
  when the test ends.
- The outage-at-deadline test needs real passes before its deadline, so
  the deadline is 30 s and each failure is asserted at or past it by the
  database clock.
- Move the credential-source withdrawal and source grant/readiness cases
  (34 tests, unchanged) into postgres-worker-agent-revision-credential-
  sources.test.mjs and register it in PostgreSQL Application.
- The SIGTERM termination test records each phase's stop time against its
  budget as a runtime-sigterm measurement in the lane result.
- A file cleanup that fails after the step's shell is gone (GitHub
  cancels a step by killing its shell, orphaning the runner) is reported
  as cleanup-cancelled instead of cleanup-failed.
… changes

- Give the controller-ahead first pass 45 s (the deadline less 15 s), not
  the default 10 s, so a stalled runner cannot fail it there.
- The lane check fails any file cleanup that did not pass, not a fixed
  list of statuses.
- Bound the new runner test's spawnSync at 30 s.
@freeqaz
freeqaz requested a review from a team as a code owner October 13, 2026 08:49
@clawsweeper

clawsweeper Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

Comment thread tests/integration/ci-runner.test.mjs Fixed
… bash -c

Code scanning flagged the bash -c wrapper as a shell command built from
paths. A node parent that waits for the runner stands in for the step's
shell just as well, and no shell parses the arguments. The test still
fails with the parent check forced off.
@clawsweeper

clawsweeper Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper status: review started.

I am starting a fresh review of this pull request: test(worker): harden the convergence deadline tests and split the revision file This is item 1/1 in the current shard. Shard 0/1.

This temporary status tracks the active review worker. The completed review will appear in the durable ClawSweeper review comment.

Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted.

This branch had an error being deployed

1 failed deployment
integration-qa-pr — 637bd9b0 Deployed Oct 13, 2026 by freeqaz via QA advisory (compose) #602
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