Skip to content

Remove now() from Clock trait#1

Closed
mircea-temp wants to merge 1 commit intomfateev:task/codex-temporalfrom
mircea-temp:remove-clock-now
Closed

Remove now() from Clock trait#1
mircea-temp wants to merge 1 commit intomfateev:task/codex-temporalfrom
mircea-temp:remove-clock-now

Conversation

@mircea-temp
Copy link

Summary

  • Remove now() from the Clock trait, SystemClock, and TemporalClock — it was dead code forced by the trait contract
  • TemporalClock::now() called Instant::now() (real system clock) inside a deterministic workflow, but nothing in the workflow path used it
  • parallel.rs now calls Instant::now() directly since ToolCallRuntime only runs outside the deterministic sandbox

Test plan

  • cargo check passes for both codex-core and codex-temporal
  • cargo test passes for codex-temporal (73 unit tests + e2e)
  • codex-core entropy tests pass

TemporalClock::now() called Instant::now() (the real system clock) inside
a deterministic Temporal workflow, but nothing in the workflow path
actually used it. The only caller was ToolCallRuntime in parallel.rs,
which is replaced by TemporalToolHandler under Temporal.

Remove now() from the Clock trait, SystemClock, and TemporalClock.
parallel.rs now calls Instant::now() directly.
mircea-temp pushed a commit to mircea-temp/codex that referenced this pull request Feb 26, 2026
…i#10590)

## Summary
This PR makes app-server-provided image URLs first-class attachments in
TUI, so they survive resume/backtrack/history recall and are resubmitted
correctly.

<img width="715" height="491" alt="Screenshot 2026-02-12 at 8 27 08 PM"
src="https://github.com/user-attachments/assets/226cbd35-8f0c-4e51-a13e-459ef5dd1927"
/>

Can delete the attached image upon backtracking:
<img width="716" height="301" alt="Screenshot 2026-02-12 at 8 27 31 PM"
src="https://github.com/user-attachments/assets/4558d230-f1bd-4eed-a093-8e1ab9c6db27"
/>

In both history and composer, remote images are rendered as normal
`[Image #N]` placeholders, with numbering unified with local images.

## What changed
- Plumb remote image URLs through TUI message state:
  - `UserHistoryCell`
  - `BacktrackSelection`
  - `ChatComposerHistory::HistoryEntry`
  - `ChatWidget::UserMessage`
- Show remote images as placeholder rows inside the composer box (above
textarea), and in history cells.
- Support keyboard selection/deletion for remote image rows in composer
(`Up`/`Down`, `Delete`/`Backspace`).
- Preserve remote-image-only turns in local composer history (Up/Down
recall), including restore after backtrack.
- Ensure submit/queue/backtrack resubmit include remote images in model
input (`UserInput::Image`), and keep request shape stable for
remote-image-only turns.
- Keep image numbering contiguous across remote + local images:
  - remote images occupy `[Image mfateev#1]..[Image #M]`
  - local images start at `[Image #M+1]`
  - deletion renumbers consistently.
- In protocol conversion, increment shared image index for remote images
too, so mixed remote/local image tags stay in a single sequence.
- Simplify restore logic to trust in-memory attachment order (no
placeholder-number parsing path).
- Backtrack/replay rollback handling now queues trims through
`AppEvent::ApplyThreadRollback` and syncs transcript overlay/deferred
lines after trims, so overlay/transcript state stays consistent.
- Trim trailing blank rendered lines from user history rendering to
avoid oversized blank padding.

## Docs + tests
- Updated: `docs/tui-chat-composer.md` (remote image flow,
selection/deletion, numbering offsets)
- Added/updated tests across `tui/src/chatwidget/tests.rs`,
`tui/src/app.rs`, `tui/src/app_backtrack.rs`, `tui/src/history_cell.rs`,
and `tui/src/bottom_pane/chat_composer.rs`
- Added snapshot coverage for remote image composer states, including
deleting the first of two remote images.

## Validation
- `just fmt`
- `cargo test -p codex-tui`

## Codex author
`codex fork 019c2636-1571-74a1-8471-15a3b1c3f49d`
mircea-temp added a commit to mircea-temp/codex-temporal that referenced this pull request Feb 26, 2026
Companion to mfateev/codex#1 which removes now() from the Clock trait.
TemporalClock::now() called Instant::now() — the real system clock —
inside a deterministic workflow. Nothing in the workflow path used it.
mircea-temp added a commit to mircea-temp/codex-temporal that referenced this pull request Feb 26, 2026
Companion to mfateev/codex#1 which removes now() from the Clock trait.
TemporalClock::now() called Instant::now() — the real system clock —
inside a deterministic workflow. Nothing in the workflow path used it.
@mfateev
Copy link
Owner

mfateev commented Feb 26, 2026

I've completely removed the Clock trait as it is not used by the code that is called in the workflow context.

@mfateev mfateev closed this Feb 26, 2026
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