Skip to content

Fix clock-sensitive smoke recording and replay - #668

Open
myevolve wants to merge 2 commits into
dependabot:mainfrom
myevolve:fix/replay-recorded-clock
Open

myevolve wants to merge 2 commits into
dependabot:mainfrom
myevolve:fix/replay-recorded-clock

Conversation

@myevolve

@myevolve myevolve commented Oct 7, 2026 •

Copy link
Copy Markdown

Problem

HTTP caches preserve registry metadata but not the clock used to evaluate release age. The same native npm input selected a different version after its cooldown expired. This is the replay-time boundary documented in dependabot/smoke-tests#592, separate from the resolver/writer policy correction in dependabot/dependabot-core#16492.

Change

  • Capture an optional input.recorded-at for new output recordings; propagate explicit timestamps through update, graph and test; preserve the timestamp in replay output.
  • Install small per-container Ruby Time.now and JavaScript Date preloads, inherited by native npm. Use a shell wrapper so existing RUBYOPT/NODE_OPTIONS remain intact.
  • Keep legacy fixtures without timestamps and ordinary non-recording updates on the host clock. Leave monotonic timers and TLS verification on real time; no production cooldown policy or updater-image dependency changes.
  • Add one actual CLI/Ruby/npm regression and document the format and migration boundary.

Verification

  • Failing-before/passing-after native regression: the Oct 2 input previously reported the Oct 7 Ruby clock and selected npm version 1.1.0. It now selects 1.0.0, a later Oct 7 input selects 1.1.0, and replaying the original selects 1.0.0 again. The test also exercises running timers and unchanged explicit-date parsing.

  • Standalone CLI with the real unmodified Core updater image, Node 24.21.0, npm 11.19.0, a stable local source revision, and cached public metadata:

    Recording clock Declared follow-redirects version Written lockfile version
    2026-10-02T06:10:32Z 1.16.0 1.16.0
    2026-10-07T06:10:32Z 1.16.1 1.16.1
    Replay of the original 1.16.0 1.16.0

    Original and replay YAML are byte-identical (7,680 bytes); replay used 20/20 cached HTTP calls. Automatic timestamp capture, legacy JSON replay without a timestamp, and ordinary unpinned updates were also exercised. A separate offline runtime check preserved existing Ruby/Node options, Ruby timezone keywords, and Date constructors/subclasses.

  • Passed: go vet ./..., go mod tidy -diff, go build ./..., and go test -shuffle=on -count=2 -race -cover -timeout=15m -parallel=2 ./... across all five packages. The exact CI command first exhausted its five-minute deadline during local Docker fixture setup; the same complete repeated race suite passed with the local envelope above. CI configuration is unchanged.

  • Two independent static reviews found no actionable defects.

Boundary

This does not rewrite the three existing smoke fixtures or claim they are now green. They need re-recording with a consistent updater and captured time; the separately documented native ignored-version cap limitation is not changed. Other runtimes' clocks and Ruby time APIs other than Time.now are not overridden.

Integration with v1.94.0

Updated to 49b4c7532aa9fbeb59b300432713bd1b5ed859ea by merging e166a3ed59c7d96aca1f4e12c64478ff72beb224 from main, including #667's container-isolation changes. The merge was clean; no isolation controls, capabilities, seccomp checks, or setup-error handling were removed or relaxed.

  • go test -race ./... passed across all five packages; the standalone CLI build passed.
  • Actual npm, npm-group-rules and npm-group-semver replays against the integrated Core corrections reproduced all three published fixtures byte-for-byte, with 202/202, 200/200 and 249/249 HTTP calls cached. Recorded clocks, versions and lockfiles remain unchanged.
  • The downloaded v1.94.0 darwin-arm64 release binary was exercised separately with the same npm fixture, Core runtime and cache. Its event comparison exits successfully, but generated YAML drops input.recorded-at and updater logs use the host clock. That release does not satisfy this PR's replay-clock contract; this is why complete-output comparison remains necessary.

The dependent migration remains dependabot/smoke-tests#593. This PR changes neither its ignore caps nor production cooldown policy. Maintainer review and a subsequent release containing this clock change are still required.

@myevolve

myevolve commented Oct 7, 2026 •

Copy link
Copy Markdown
Author

Current head: 49b4c7532aa9fbeb59b300432713bd1b5ed859ea

104/104 checks passed, including 2/2 required checks; no failed or pending checks. Required build and lint both pass. This supersedes the earlier head's three optional smoke failures.

The branch cleanly integrates released CLI v1.94.0 / main e166a3ed59c7d96aca1f4e12c64478ff72beb224, including #667's updater-container isolation. No isolation control was weakened.

Local verification also passed:

  • go test -race ./... across all five packages, followed by a CLI build.
  • Three actual replays of smoke-tests #593 at aeed1cf2ffe310b228b69cc493de09182455372f, using integrated Core #16492 + Core #16496: complete outputs are byte-identical, with npm 202/202, rules 200/200, and semver 249/249 HTTP calls cached. Recorded clocks, dependency versions and lockfiles are unchanged; no metadata or job errors.

The actual downloaded v1.94.0 release binary was exercised separately with the same npm fixture/runtime/cache. Its event comparison exits 0, but the generated YAML drops input.recorded-at (the only complete-output difference) and the updater uses the host clock. Thus the new release does not yet satisfy this PR's replay-clock contract.

Maintainer review is still required. The dependent fixture PR remains a draft pending CLI/Core releases, official cache refreshes and released-toolchain verification. No approval, merge or release was attempted. No write-enabled host credential was exposed to the updater/proxy.

Core-main follow-up: Latest runtime heads are Core #16492 97c3cac44b62df8ecf93b08f9041849e85ffa20f and Core #16496 de4a7cb8bf466b2667fdb4b8bc5948a1061d86c2, both incorporating main 0286aa0868b1f0de1b908ce3358bbf994e5272c1. The merge changes shared notice rendering and Julia stdlib notices, without weakening npm policy guards or fixture contracts. Full-repository Sorbet passed on all three refreshed Core branches and again after the policy spec-isolation correction. Direct shared-notice rendering confirms INFO/fallback → NOTE, WARN → WARNING, ERROR → IMPORTANT. Three complete CLI replays at cooldown 97c3cac44b62df8ecf93b08f9041849e85ffa20f + policy 260f77e6e25e0b6cbcf00882ada974398de69a82 are byte-identical, with 202/202, 200/200 and 249/249 cached calls, unchanged clocks and no metadata/job errors. Policy’s subsequent de4a7cb8bf466b2667fdb4b8bc5948a1061d86c2 commit changes only spec isolation and its README; all mounted runtime library trees are unchanged. CLI source is unchanged; no new Go-suite run is claimed.

Review-routing correction: REST GET /repos/{owner}/{repo}/pulls/{number}/requested_reviewers confirms that dependabot/maintainers is already requested on Core #16492, #16496, #16498 and CLI #668. CLI #668 also requests dependabot/azure-dev-ops. These requests predate this continuation: Core requests are dated October 7/8 and both CLI requests October 7. gh pr view displayed empty reviewer lists, while GraphQL reports the existing requests with requestedReviewer: null; the empty display did not mean reviews were unassigned. Core REST evidence; CLI REST evidence.

The later individual requests for JamieMagee and jakecoffman returned HTTP 404 and added no individual reviewers. Existing team requests need review responses, not duplicate assignment. All four prerequisite PRs still report REVIEW_REQUIRED; the account remains read-only upstream. No new review request, approval, upstream merge, release, official cache refresh or protection bypass was performed.

@myevolve myevolve left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent AI-assisted technical review

Reviewed 49b4c7532aa9fbeb59b300432713bd1b5ed859ea with a separate clock/lifecycle reviewer: all 12 changed files and affected CLI/container/output callers. No actionable correctness findings were identified.

Parent verification: built this exact CLI head and replayed all three unchanged npm fixtures against the combined, repaired Core sources. The generated full YAML outputs are byte-identical to the committed fixtures; no input or expected-output weakening was used. The reviewer performed static review only, not those runtime checks.

This is an explicitly AI-authored technical COMMENT, not maintainer approval. It does not satisfy required independent-human reviews or publish a CLI release.

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.

1 participant