Skip to content

fix(cli): keep project replacement timestamps at or after wall clock - #5666

Merged
wesbillman merged 2 commits into
block:mainfrom
Illuminfti:pr/project-ts
Aug 17, 2026
Merged

fix(cli): keep project replacement timestamps at or after wall clock#5666
wesbillman merged 2 commits into
block:mainfrom
Illuminfti:pr/project-ts

Conversation

@Illuminfti

Copy link
Copy Markdown
Contributor

Fixes #5665.

next_timestamp in crates/buzz-cli/src/commands/projects.rs computed a replacement's created_at as head.created_at + 1. The relay's ingest path rejects events more than ±900s from server time (MAX_TIMESTAMP_DRIFT_SECS in crates/buzz-relay/src/handlers/ingest.rs), so:

Change

next_timestamp now returns max(now, head.created_at + 1): strictly after the observed head (preserving the dominate-the-head guarantee for skewed/future heads), never behind the wall clock. This mirrors the relay's own replacement-authoring pattern (now.max(head+1) in side_effects.rs).

Testing

  • cargo test -p buzz-cli --lib — 344 passed; adds next_timestamp_uses_wall_clock_when_head_is_stale, and the existing far-future-head test still holds (head+1 wins when head > now)
  • cargo clippy -p buzz-cli --all-targets / cargo fmt --check — clean
  • Live before/after on a self-hosted relay: vanilla CLI fails on a 2h-aged head; with this change the same update is accepted and the head lands at wall clock.

Same failure family as #2876 (repos protect vs the drift window) — that path is not touched here.

next_timestamp computed replacement created_at as head+1, which collides with the relay's +/-900s ingest drift window: updating any project whose head is older than 15 minutes is rejected with 'event timestamp too far from server time', and even fresh replacements are recorded seconds in the past, losing LWW races against wall-clock writers.

Use max(now, head+1) instead - strictly after the observed head, never behind the wall clock - matching the pattern the relay itself uses when authoring replacements in side_effects.rs.

Fixes block#5665

Signed-off-by: Ika Minami <ika@infiniteidol.com>
@Illuminfti
Illuminfti requested a review from a team as a code owner August 12, 2026 14:42

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 374b6143d5

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

/// Advance past the observed head without falling outside the relay window.
fn next_timestamp(head: &Event) -> Result<Timestamp, CliError> {
head.created_at
let after_head = head

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Add the required DCO sign-off

When this commit is submitted in a PR, the required DCO check will fail because its message contains no Signed-off-by trailer. Recreate the commit with git commit -s or otherwise add the sign-off before merging.

AGENTS.md reference: AGENTS.md:L112-L112

Useful? React with 👍 / 👎.

@ScaleLeanChris

Copy link
Copy Markdown

Independent verification at 374b6143d against current main:

  • focused buzz-cli Projects suite: 44 passed, 0 failed;
  • cargo clippy -p buzz-cli --all-targets -- -D warnings: passed;
  • cargo fmt --all -- --check: passed.

The implementation matches the relay's replacement ordering rule: max(now, head.created_at + 1). It remains strictly newer than an observed future head, while an aged head no longer creates an event outside the relay's 15-minute acceptance window. The change is confined to the Projects CLI timestamp helper and two focused relationship tests.

I found no blocker in this patch. A live relay reproduction would add little beyond the deterministic helper contract because the reported failure is exactly the timestamp value submitted to the unchanged relay drift check.

@ravarora2 ravarora2 added the triage-ready Appropriate for agentic review label Aug 14, 2026
Co-authored-by: Ravneet Arora <rarora@squareup.com>
Signed-off-by: Ravneet Arora <rarora@squareup.com>
@ravarora2

Copy link
Copy Markdown
Contributor

🤖 I pushed 07a7b023b9b71fa024012ae7d8650cecc17d00e9 to this PR to address the deterministic timestamp-test blocker.

What changed

  • next_timestamp now accepts an explicit now; production callers pass Timestamp::now(), while tests use fixed values.
  • The test matrix asserts exact results for a stale head, a head equal to now, a future head, the last future timestamp inside the relay's +900s window, and the first one that cannot both dominate the head and remain inside that window.
  • A dedicated u64::MAX test verifies a clean overflow error rather than wrapping.
  • The helper and command comments now describe the exact max(client_now, head + 1) rule without claiming every future head can fit the relay window.

Regression evidence

The committed tests fail under each temporary mutation below; all mutations were removed afterward:

  1. Restore the former head + 1 behavior: the stale-head case fails.
  2. Return at least now + 901: the exact stale-head assertion fails.
  3. Replace checked addition with wrapping addition: the overflow test fails.
  4. Select min instead of max: the stale-head case fails.

Verification at the pushed commit

  • cargo test -p buzz-cli: 344 unit tests passed, 0 failed; main target 0; one doctest ignored.
  • cargo clippy -p buzz-cli --all-targets -- -D warnings: passed.
  • cargo fmt --all -- --check: passed.
  • git diff --check: passed.
  • Live isolated relay: a project head was backdated by two hours, then the exact rebuilt CLI successfully updated it. The accepted replacement was timestamped 17:55:43 UTC; wall clock at read-back was 17:55:49 UTC.

The production timestamp policy is unchanged; this commit makes its contract deterministic, complete, and mutation-sensitive.

@ravarora2 ravarora2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm, will wait for owners to approve

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Princess Donut, an automated reviewer, commenting via Wes’s GitHub account.

Reviewed exact head 07a7b023b9b71fa024012ae7d8650cecc17d00e9. I found no material correctness, security, or user-trust defect in this patch.

The implementation is the smallest complete repair for the reported failure:

  • next_timestamp computes max(client_now, head.created_at + 1) with checked arithmetic (crates/buzz-cli/src/commands/projects.rs:125-136). A stale head therefore no longer emits an event older than the relay’s ±900-second ingest window, while an equal/future observed head is still strictly dominated.
  • Every project replacement surface uses the helper: add-repo, remove-repo, update, and delete (projects.rs:294-297,348-351,440-443,506-509). Create remains correctly outside this read-modify-write path.
  • Overflow produces an explicit CLI error rather than wrapping. The deterministic matrix pins stale, equal, future, relay-boundary, impossible-to-dominate-within-window, and u64::MAX behavior (projects.rs:988-1035).
  • The comments candidly preserve the unavoidable residual behavior: a sufficiently future accepted head may make the next strictly newer replacement exceed relay drift until wall clock catches up (projects.rs:7-10,125-128). The helper does not pretend it can satisfy contradictory monotonicity and relay-window constraints.

I also traced the unchanged sink: relay ingest rejects absolute drift above 900 seconds (crates/buzz-relay/src/handlers/ingest.rs:1976-1983), and relay-authored replacements use the same now.max(head+1) ordering (handlers/side_effects.rs:992-1017). The concurrency tradeoff is not newly unsafe: writes remain read-modify-write without CAS, but assigning timestamp at the observed-head fetch point means an earlier reader does not leapfrog a later reader solely because it submits later.

Exact-head CI is fully green, including Unit Tests and Rust Lint. git diff --check origin/main...HEAD passes on a clean worktree. I did not duplicate CI-equivalent suites locally.

Verdict: CLEAR. This is ready from an adversarial correctness and user-trust perspective. I am leaving a comment review rather than approving because approval was not explicitly requested.

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Consolidated Royal Court review of exact head 07a7b023b9b71fa024012ae7d8650cecc17d00e9.

No material correctness, security, concurrency, or integration defect found.

The patch implements the required ordering rule cleanly: next_timestamp computes checked head.created_at + 1, then chooses max(client_now, after_head). That fixes stale project heads producing relay-rejected or misleadingly old replacements while preserving strict domination of the observed head. Overflow fails explicitly. The rule is used by all four replacement paths: add-repo, remove-repo, update, and delete; create correctly remains outside this read-modify-write flow.

The behavior-changing tests are appropriate and deterministic. They cover stale, equal, and future heads; both sides of the relay's +900-second boundary; and u64::MAX overflow. The documented residual case is unavoidable: if the observed head is sufficiently far ahead, no timestamp can both dominate it and satisfy the relay drift window until wall clock catches up. A client clock skewed beyond the relay's existing ±900-second policy likewise remains correctly rejected.

I independently traced the unchanged relay ingest bound and the matching relay-authored max(now, head + 1) policy. Exact-head CI is green, git diff --check origin/main...HEAD passes, and focused local validation passed 44/44 buzz-cli project tests.

The PR base predates Projects v3, which materially changed the same file, so I also trial-merged current origin/main at 85bacea52b8359999f22c6ac07207a130809c488. Git merged it cleanly, retained the helper and all four call sites, and the merged focused project suite passed 44/44. I aborted the merge and restored the clean exact-head worktree.

Verdict: CLEAR. I am leaving a comment review rather than approving because Wes did not explicitly request approval.

@wesbillman
wesbillman merged commit a282e06 into block:main Aug 17, 2026
31 checks passed
wpfleger96 pushed a commit that referenced this pull request Aug 17, 2026
…gaps

* origin/main:
  fix(desktop): align preview sidebar row styling (#6163)
  fix(desktop): repair dropped team membership links at boot and on edit (#5904)
  fix(cli): keep project replacement timestamps at or after wall clock (#5666)
  Remove GitHub security advisory commitment (#6144)
  Rename Bumble agent to Pollen (#5864)
  fix(desktop): resolve agent profiles through one archive-aware selector (#5706)
  fix(acp): gate relay-signed workflow messages on their attributed author (#6129)
  fix(acp): replace Goose native system prompt (#5964)
  feat(workflows): add responsive library card actions (#6008)
  fix(desktop): enforce shared agent access across devices (#6086)

Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
morgmart added a commit that referenced this pull request Aug 18, 2026
…graphy-staging

* origin/main:
  test(desktop): cover exact workflow batch limit (#6168)
  chore(release): release Buzz Desktop version 0.5.15 (#6173)
  Preserve managed agent mentions during relay errors (#6167)
  fix(workflows): preserve multi-channel listing semantics (#6009)
  Remove Startup Recovery section in base prompt (#6161)
  fix(desktop): align preview sidebar row styling (#6163)
  fix(desktop): repair dropped team membership links at boot and on edit (#5904)
  fix(cli): keep project replacement timestamps at or after wall clock (#5666)

Signed-off-by: morgmart <98432065+morgmart@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

triage-ready Appropriate for agentic review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

projects update fails on any project older than 15 minutes (CLI head+1 timestamp vs relay drift window)

4 participants