CRW-567: give the crw recall CLI a link-time clock seam and replay the nine clock fixtures - #556
Merged
Merged
Conversation
…play contract/notes/cxc/CRW-567.json claims cli__chat__natural_language_query_relaxed, cli__chat__search_plain_envelope, cli__chat__search_refresh_builds_index, cli__chat__search_scan_json_and_plain, cli__memory__search_chat_fallback_and_no_chat, cli__memory__search_cwd_boost_filter_and_origin_federation, cli__memory__search_markdown_and_db, cli__memory__search_plain_envelope and cli__memory__search_status_text_and_json: four identical, five intentionally changed on the step stdout because the recorder's clock advanced 1 ms per Date read and the recency-decayed hits[].score differs in the 1e-9 digits. The nine ids leave CRW-391.json's pending list, which keeps only the allow-write help. The replay fails without the recall clock seam.
cmd/crw/recall_clock.go adds recallTestClock (a string var with no initializer; -X fills it in a test build only) and recallNow() (the wall clock when empty, else the frozen instant in UTC; a malformed value panics with the value it read). The recall row of cmd/crw/main.go now calls recallNow(). The cxc domain of internal/contracttest builds its own crw with the recorder's 2026-01-01T00:00:00Z linked in (cxcRecallBinary, prebuilt in TestMain) while every other domain keeps testsupport.CRWPath, so the nine clock-dependent fixtures replay against the recording. cmd/crw/recall_clock_test.go covers the empty seam, the frozen instant, the panic and a release build reading the wall clock. contract/notes/cxc/README.md's Seams section says the recall CLI now replays under the link-time clock; the network log and V8 stack text stay unprovided.
|
You have reached your Codex usage limits for security reviews. Please try again later. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
The nine clock-dependent fixtures of the CXC replay corpus (contract/fixtures/cxc) could not leave pending: the recall CLI read the wall clock, while the recording was made under a frozen clock at 2026-01-01T00:00:00Z that advanced 1 ms per Date read (contract/schema/cxc/normalisation.json). This PR adds a link-time clock seam to the recall CLI and links the recorded instant into the cxc domain's own replay binary, so the nine fixtures replay and are claimed in contract/notes/cxc/CRW-567.json.
Acceptance criteria: the nine fixture ids are no longer pending in contract/notes/cxc/CRW-391.json; TestDomain/cxc replays them green under the frozen clock; go test ./cmd/crw/ ./internal/recall/... and the contracttest package pass; the existing corpus, goldens and tests are unchanged; the activation surface (skills, wiring/hooks, plugin manifest) is untouched and crw-dev ci plugin reports the same digest.
Changes
Oracle sources
No CXC source is ported by this PR: the recall CLI and its injected clock (internal/recall/cli.go, Run(args, stdout, stderr, now time.Time)) already exist. What this PR ports is the recorder's clock contract from contract/schema/cxc/normalisation.json ("The oracle also runs under a frozen clock (2026-01-01T00:00:00.000Z, +1 ms per Date reading) with TZ=UTC") into a link-time seam, following the existing precedent internal/relay/adapter/cli.go (testClock).
Fixture classification
All nine are replayed by the cxc domain entry point "go test -count=1 -run 'TestDomain/cxc' ./internal/contracttest" (one subtest per fixture, driven through crw recall chat search / crw recall memory search / crw recall memory status CLI steps):
Fixtures this issue's units could drive but that stay pending under their own issues: cli-help__memory__allow-write_dashdash_help, the hook-path fixtures, the bare memory help fixture and the background (bg) help fixtures.
The score difference (why five claims are intentionally changed)
The recorder's clock advanced 1 ms per Date read, so a decayed score's last digits depend on how many clock reads preceded it (recorded 15.499999998280884, replayed with the fixed instant 15.5; recorded 4.375506063230572, replayed 4.3755060648070065). A dump of the full flattened observation and expectation for the five showed score-only differences: 1, 1, 4, 2 and 3+3+2 and 2+1 score fields, and zero differences at any other path. Nothing outside hits[].score differs.
Verification (this head)
All local Go commands run with TZ=UTC (the host's KST makes the recorded-oracle test in internal/recall fail at the untouched baseline too; see Defects):
Red before, green after: with the nine claims in place and the frozen clock not linked, TestDomain/cxc fails exactly those nine fixtures (exit 1); with the link it passes. The first red state (claims in place, before the seam existed at all) failed the same nine.
Size
Counted lines (implementation + tests, added + deleted, generated data excluded): 149. Generated data: contract/notes/cxc/CRW-567.json, 50 lines of recorded claim data. Whole diff: 8 files, 184 insertions, 15 deletions.
Defects
No new defect line: the nine fixtures needed no CXC behaviour change and this unit found no defect in the ported behaviour. One pre-existing local environment sensitivity surfaced while running the verifiers and is already recorded in docs/port-cxc/known-defects.md (zone-less legacy stamps parse in local time, so the recorded-oracle test in internal/recall must run under TZ=UTC). It is not touched here.
Out of scope
The hook path, the allow-write help fixture, the bare memory help and the bg fixtures; the activation surface (skills, wiring/hooks, plugin manifest); installation, service activation and live relay behaviour.