Skip to content

Get rid of unneeded DateTimeExt.#11976

Merged
vorporeal merged 2 commits into
masterfrom
david/get-rid-of-unneeded-datetime-ext
Jun 1, 2026
Merged

Get rid of unneeded DateTimeExt.#11976
vorporeal merged 2 commits into
masterfrom
david/get-rid-of-unneeded-datetime-ext

Conversation

@vorporeal
Copy link
Copy Markdown
Contributor

@vorporeal vorporeal commented May 31, 2026

Description

This extension trait adds a pretty low-value helper function; I've replaced it with the slightly-more-verbose-but-more-explicit equivalents using chrono functions.

I double-checked that some of these conversions were truly equivalent by using assertions in the Rust playground.

@cla-bot cla-bot Bot added the cla-signed label May 31, 2026
Copy link
Copy Markdown
Contributor Author

vorporeal commented May 31, 2026

Base automatically changed from david/create-warp_server_auth-crate to master June 1, 2026 18:42
@vorporeal vorporeal force-pushed the david/get-rid-of-unneeded-datetime-ext branch from ba90a52 to 4330f49 Compare June 1, 2026 18:47
@vorporeal vorporeal requested a review from bnavetta June 1, 2026 18:53
@vorporeal vorporeal marked this pull request as ready for review June 1, 2026 18:54
@oz-for-oss
Copy link
Copy Markdown
Contributor

oz-for-oss Bot commented Jun 1, 2026

@vorporeal

I'm starting a first review of this pull request.

You can view the conversation on Warp.

I completed the review and no human review was requested for this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

Copy link
Copy Markdown
Contributor

@oz-for-oss oz-for-oss Bot left a comment

Choose a reason for hiding this comment

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

Overview

This PR removes the DateTimeExt helper and replaces its call sites with direct chrono::Local::now() or .fixed_offset() usage.

Concerns

  • No blocking correctness, security, or spec concerns found in the annotated diff.

Verdict

Found: 0 critical, 0 important, 0 suggestions

Approve

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

@vorporeal vorporeal enabled auto-merge (squash) June 1, 2026 19:21
@vorporeal vorporeal merged commit 6616a42 into master Jun 1, 2026
26 checks passed
@vorporeal vorporeal deleted the david/get-rid-of-unneeded-datetime-ext branch June 1, 2026 19:36
vorporeal added a commit that referenced this pull request Jun 2, 2026
… crate. (#11977)

## Description
This PR is the authentication slice of a broader effort to move server
API client traits and implementations out of the `warp` application
crate and into `warp_server_client`. It completes the intended
extraction of `AuthClient` and reusable authentication-session behavior
for this phase: shared server clients can now consume auth functionality
without depending on app-owned `ServerApi` implementation details, while
UI/model reactions and app-specific conversions remain in `warp`.

This PR is stacked on #11976 (`david/get-rid-of-unneeded-datetime-ext`).
It intentionally changes ownership rather than behavior; extraction of
non-auth server clients is left to future PRs.

### Architectural decisions
- `warp_server_client::auth` now owns `AuthClient`, its implementation,
and the reusable authentication-session mechanics used by server
clients.
- `AuthSession` represents the stateful mechanics of authenticating
requests: exchanging login credentials, fetching or refreshing Firebase
tokens (including proxy fallback), completing OAuth device
authorization, and producing usable access tokens. It owns mechanics,
not product reactions.
- `AuthEvent` is the lower-crate event vocabulary for authentication and
authenticated-transport conditions observed while making requests. The
app continues to decide what those events mean for models/UI, such as
setting reauth state, rotating remote-server tokens, showing staging
access feedback, logging out disabled users, or responding to IAP
challenges.
- `AuthClientImpl` is constructed directly by `ServerApiProvider` around
the shared app transport adapter and `AuthSession`; the app no longer
implements a forwarding `AuthClient` wrapper.
- `BaseClient` supplies the app-owned transport/platform capabilities
still required by the extracted client (HTTP setup, GraphQL request
options, ambient workload token behavior, and notification hooks),
rather than owning token/OAuth lifecycle operations.
- `UserProperties` conversion remains in `warp` because it maps protocol
user output into app models, server experiment state, and app LLM
preferences rather than describing reusable server authentication
behavior.
- The shared GraphQL request helper and its behavior tests now live in
`warp_server_client`, so authentication rejection/event behavior is
tested at the owning layer.
- The `skip_login` build-time behavior is forwarded into
`warp_server_client` because authenticated-request rejection is now
implemented by `AuthSession`.

### Reviewer notes
- This is the completed auth extraction slice of an incremental
server-client boundary migration; non-auth `ServerApi` methods
intentionally remain app-owned for later work.
- Network-log client instrumentation is installed before the HTTP client
is shared with `AuthSession`, preserving the existing initialization
invariant while allowing the extracted session to reuse that client.
- Existing comments associated with moved behavior were restored at
their new ownership locations; app-specific comments that no longer
describe the extracted code were not carried over.
- A follow-up review pass restored operation-specific settings error
diagnostics, added focused extracted-auth tests, and moved the app's
remaining direct Firebase dependency to test-only usage.

## Linked Issue
N/A — internal refactor in a stacked extraction series.

## Testing
- `./script/format`
- `cargo check -p warp_server_client -p warp --tests`
- `cargo nextest run -p warp_server_client --no-tests=pass`
- `cargo nextest run -p warp -E
'test(/auth_manager|server_api::auth|server_api::ai|settings::privacy|settings_view::platform|terminal::shared_session::viewer/)'
--no-tests=pass`
- `cargo nextest run -p warp --features skip_login -E
'test(access_token_skip_login_rejects_bearer_token)' --no-tests=pass`
- `cargo nextest run --no-fail-fast --workspace --exclude
command-signatures-v2`
- `cargo clippy --workspace --exclude warp_completer --all-targets
--tests -- -D warnings`
- `PATH="$(brew --prefix llvm)/bin:$PATH" cargo clippy --locked --target
wasm32-unknown-unknown --profile release-wasm-debug_assertions -- -D
warnings -A clippy::large_digit_groups`
- `git --no-pager diff --check`

- [ ] I have manually tested my changes locally with `./script/run`

## Agent Mode
- [x] Warp Agent Mode - This PR was created via Warp's AI Agent Mode

CHANGELOG-NONE

Co-Authored-By: Oz <oz-agent@warp.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants