Skip to content

ui-server: consolidate duplicate cron recorder success and error cases #2976

Open

Description

@schlessera

Context

Part of #1849. Source-verified finding from the 2026-10-25 multi-agent test-value audit at c670449d3bf1b0639e50ce554d728961f4313add. Broad inventory screening was followed by detailed inspection of this finding; this is not a claim of exhaustive manual review or measured CI savings.

Classification: confirmed same-function duplicate coverage; small maintenance improvement.
Suggested issue title: Consolidate recordCronRun success/error cases without losing deterministic timing proof.

Evidence:

  • packages/ui-server/tests/cron-scheduler.test.ts:41-60, “records a successful job run,” calls recordCronRun directly; it does not exercise createCronScheduler.triggerJob.
  • :62-74, “records a failed job run with error message,” also calls recordCronRun directly.
  • Same file :143-151, “finishing without an error writes the success shape runJob writes,” and :153-160, “finishing with an error writes the error shape,” call the same production function again in a separate in-memory fixture.
  • Production function is packages/ui-server/src/cron/scheduler.ts:49-69. Success/error rows are the two finish branches.
  • First describe's setup creates an on-disk WAL DB at :14-31 for every test. Second describe uses an in-memory DB at :110-123.
  • Real surrounding runtime proof remains packages/ui-server/tests/cron-bin.test.ts:52-67 (“run records a row and returns the child's exit code”) and cron-run-job.test.ts's independent wrapper/error/cleanup cases.

Unique assertion:
The first success case mocks Date.now across a 1500 ms interval and checks the exact finished_at. The second success case only checks duration equals finished_at minus started_at; removing the first without transfer loses a deterministic nonzero-duration control. Error message literals differ, but no distinct branch is introduced.

Safe change:
Move the exact controlled-clock success assertions into the in-memory recorder suite, retain one success and one error case, and remove the first pair. Keep getCronStatus latest/multiple/empty tests, running-between-start-and-finish and concurrent-job non-cross-write tests. Use the shipped schema or preserve separate shipped-schema proof; do not treat a handwritten fixture as migration integration.

Acceptance criteria:

  1. One recorder success case checks exact nonzero duration, finish time, status and null error.
  2. One recorder error case checks literal error/status/duration.
  3. Running state and cross-job isolation remain.
  4. Status reading and actual cron binary/wrapper proof remain.
  5. Duration/status/error mutants each fail the named consolidated assertion.

Focused verification/mutation proposal:
bun run test ./packages/ui-server/tests/cron-scheduler.test.ts plus focused cron-bin “run records a row” when modifying runtime wiring. A disposable isolated-source mutant writing zero duration or success on error must fail consolidated recorder assertions. No test or mutation was run for this pair in the audit.

Count/cost:
Two duplicate test bodies and their two on-disk WAL fixture lifecycles can be removed. Small, unmeasured cost; do not call it a meaningful CI bottleneck.

Scope

Consolidate the two recorder success/error pairs into one cheap fixture, transferring deterministic clock assertions and preserving runtime scheduler/bin proof.

Out of scope

Changing product behavior, public contracts, required proof gates, offline guards, runtime pins or unrelated tests. Every distinct native, security, lifecycle and adapter obligation identified above remains. This issue does not authorize deleting the surviving owner named by another audit issue.

Acceptance criteria

  • One success test pins nonzero duration, exact finish time, status and null error; one error test pins literal error, status and duration.
  • Remove two duplicate on-disk recorder fixture lifecycles while retaining running-state, cross-job isolation, status-reader and real cron-bin/wrapper checks.
  • Record an assertion-to-owner mapping for every moved or removed case, including shared-helper obligations; do not count matching test names as proof of equivalence.
  • Run the proposed focused production mutation(s), observe failure on the intended assertion, restore, and record passing focused proof. A module-load error is not a behavioral receipt.
  • Any performance claim separates scenario/body cost, setup and total elapsed time and uses comparable observations; structural counts alone are not a speedup.

Verification

bun run test ./packages/ui-server/tests/cron-scheduler.test.ts ./packages/ui-server/tests/cron-bin.test.ts ./packages/ui-server/tests/cron-run-job.test.ts
bun run check:pr --base origin/main

Preserve the existing offline/keyless harness. Browser/native commands require the documented real runtime and OS-offline prerequisites; missing prerequisites are not passing or skipped proof. The mutation proposals and retained boundary checks are detailed above.

Contract impact

No integration-contract or product-behavior change is selected. This is test/proof maintenance; retain the current CLI/MCP/wire/schema/frontmatter envelopes. No release-content change is selected.

Activity

  1. added
    type: testCoverage, harness or proof work with no behaviour change.
    area: ui-serverHono app factory: turn coordinator, auth, session catalog, routes, cron.
    priority: p2Wanted, not scheduled. Pulled in when a milestone has room.
    agent-readyContext and acceptance criteria are complete enough for an agent to take this unattended.
    on Oct 25, 2026
  2. schlessera commented on Oct 25, 2026

    @schlessera
    OwnerAuthor

    2026-10-26 triage at main c670449d3bf1b0639e50ce554d728961f4313add: verified the cited current test/production boundary and native #1849 parent. The scope names the retained proof, exclusions, checkable acceptance criteria, focused verification and mutation requirements; no unresolved design, decision, access or reproduction prerequisite prevents starting this test-maintenance task. Set priority p2 for wanted, unscheduled work and add agent-ready. Keep native/browser/SDK qualification required by the brief as implementation proof; this source/readiness review supplies no completed repair, post-change equivalence or measured CI saving.

  3. added this to the 0.44.0 milestone on Oct 29, 2026
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

    agent-readyContext and acceptance criteria are complete enough for an agent to take this unattended.area: ui-serverHono app factory: turn coordinator, auth, session catalog, routes, cron.priority: p2Wanted, not scheduled. Pulled in when a milestone has room.type: testCoverage, harness or proof work with no behaviour change.

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions