Skip to content

fix(environment): retain ID ordering with missing clocks - #8957

Open
xu91102 wants to merge 2 commits into
morluto:mainfrom
xu91102:codex/environment-sparse-clock-order
Open

xu91102 wants to merge 2 commits into
morluto:mainfrom
xu91102:codex/environment-sparse-clock-order

Conversation

@xu91102

@xu91102 xu91102 commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Problem and resulting behavior

Fixes #8951. When existing creation clocks are missing, environment-build treated an unknown comparison as evidence that IDs were random, changing learned create insertion from ascending ID to append. The shared randomIdentities evidence owner now requires a witnessed inversion among canonical clock readings. Missing or invalid observations do not hide inversions between the remaining known values; seconds/milliseconds and submillisecond precision compare as instants.

The concrete Greptile P1 on the first head exposed an inversion across a missing middle clock. The repair is delivered in a3e16d1 and also covers invalid text and mixed-unit gaps. UUID-v4 classification, all-clock qualification, the exported comparator, insertion candidate ranking, generators, later-list checks, refusal/rollback, responses and cleanup retain their owners and behavior. No new parser/schema/dependency, target execution or retry/timeout changes.

Own real CLI reproduction was recorded in issue comment6648112176 before implementation. Related production sort-learning changes were read; no other same-root public implementation was found at publication. No foreign runtime is used as validation.

Validation

  • Final eight complete focused files:308PASS; fresh README9 => nine distinct317PASS0skip. Six real HTTP MCP scenarios create three items and read a six-item list; their24 calls are within those six tests. Existing write validation and rollback consumers are included.
  • Seven immutable HAR cases through actual compiled and normally installed public CLIs:14 calls per product. Source hashes unchanged; controls and gap outcomes match. These calls are separate from the317 tests.
  • Full check, docs, discovery3029files exactly once, explicit pack dryrun and official installed-package verification passed:234tools/sixprompts, exact6.8.0 update, verified process lineage and awaitedcleanup. All2781 archive/current/installed published files match.
  • Independent frozen-package Standards and Spec v6 both PASS. Earlier failed probes/reviews are retained; their counts are not added to final results.
HAR case              final compiled / installed insertion
sparse / complete     sorted id ascending
no creation clock     sorted id ascending
inverted clock        sorted created_at descending
missing-clock gap     last
invalid-text gap      last
seconds/ms gap        last

macOS arm64, Node24.22.1. Captures are locally generated offline HAR inputs with real local HTTP serving; external application/browser capture, complete provider/agent/model workflows, other hosts and performance were not run. Current repaired-head upstream CI/Codex remains separate from these local results. No migration or breaking schema change.

Author readiness

Human owner: @xu91102

  • I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
  • I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
  • I declare this PR ready for initial maintainer review.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-11-05T03:21:37.585426Z
We couldn’t complete this review. Comment @codex review to try again.
34876bb PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

@greptile-apps

greptile-apps Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; the previous ordering defect is fixed and no new blocking issues were found.

Summary

Keeps ID ordering when captured clocks are missing, while still recognizing real inversions across gaps.

  • Missing clocks no longer make ordered IDs look random.

Reviews (2) · Last reviewed commit: "fix(environment): compare known creation..." · Reviewed by Greptile

This branch has not been deployed

No deployments
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.

[Bug] environment-build demotes ID sorting when existing creation clocks are missing

1 participant