REG-1909 Contract Preview 2b

[sirdodger]

REG-1909 Contract Preview 2b

Crate-implementation PR (stacked on REG-1909 Foundation 1) for rover contract preview.

Adds operations::contract::preview: the contractPreviewAsync/contractPreviewStatus/contractPreviewResult GraphQL operations, each wrapped as a tower::Service per AGENTS.md's rover-client Service pattern (see contract/preview/service.rs), plus start/result/poll/run orchestration functions built on the shared preview_poll helper added in the foundation PR.

No CLI command calls this yet — rover contract preview wiring lands in the next PR in this stack (REG-1909 Contract Preview 3b).

[x] A CHANGELOG.md entry is not needed for this PR

[apollo-librarian]

✅ Docs preview has no changes

The preview was not built because there were no changes.

Build ID: bac257d328a73239f436545a
Build Logs: View logs

---

✅ AI Style Review — No Changes Detected

No MDX files were changed in this pull request.

Review Log: View detailed log

This review is AI-generated. Please use common sense when accepting these suggestions, as they may not always be accurate or appropriate for your specific context.

[claude]

Code review

Two findings, both against the retry/timeout composition guidance in CLAUDE.md. No correctness bugs found — GraphQL variables, union/enum mapping, and poll polarity all check out against the checked-in schema.

---

1. poll has no per-attempt TimeoutLayer nested inside its retry

rover/crates/rover-client/src/operations/contract/preview/mod.rs

Lines 130 to 147 in 6702205

	let build_id = status_input.build_id.clone();
	let mut status_service = ServiceBuilder::new()
	.retry(PollRetryPolicy::new(
	Duration::from_secs(5),
	Duration::from_secs(checks_timeout_seconds),
	{
	let build_id = build_id.clone();
	move || RoverClientError::PreviewTimeoutError {
	build_id: build_id.clone(),
	}
	},
	))
	.service(service::ContractPreviewStatus::new(
	client
	.studio_graphql_service()
	.map_err(|err| RoverClientError::ServiceReady(Box::new(err)))?,
	));
	status_service

poll wraps ContractPreviewStatus in a PollRetryPolicy retry, but nothing bounds an individual attempt.

PollRetryPolicy::retry() implements tower::retry::Policy::retry, which Tower only invokes after the inner call has already resolved to a Result. The deadline comparison lives entirely inside that method, so if a single contractPreviewStatus request hangs, retry() is never reached and the checks_timeout_seconds budget is never enforced — the poll can block indefinitely instead of surfacing PreviewTimeoutError.

CLAUDE.md is explicit about this shape:

nest a short per-attempt TimeoutLayer inside a RetryLayer whose retry budget is the longer elapsed-time window, so a single hung attempt can’t consume the entire retry budget.

Suggested fix: add a rover-http TimeoutLayer with a short per-attempt timeout between .retry(...) and .service(...), so each status call is bounded while PollRetryPolicy keeps owning the overall elapsed-time budget. src/command/auth/whoami/mod.rs is the in-repo reference for the retry-outer / timeout-inner ordering.

For fairness: the pre-existing shared/check_workflow_poll.rs has the same structural gap in its manual loop — but this is new ServiceBuilder-based code, which is exactly what the rule targets.

---

2. The new operation services expose no default timeout/retry configuration

rover/crates/rover-client/src/operations/contract/preview/service.rs

Lines 18 to 29 in 6702205

	/// A [`Service`] that starts an async contract preview build, layered over
	/// the studio GraphQL service.
	#[derive(Clone)]
	pub struct ContractPreviewStart<S: Clone> {
	inner: S,
	}
	impl<S: Clone> ContractPreviewStart<S> {
	pub const fn new(inner: S) -> Self {
	Self { inner }
	}
	}

None of the three new wrappers — ContractPreviewStart (L21), ContractPreviewStatus (L87), ContractPreviewResult (L155) — expose a default timeout/retry configuration for callers to use or override.

CLAUDE.md states this without hedging:

Each rover-client operation’s Tower Service should expose its own default timeout/retry configuration, not just inherit a one-size-fits-all constant. A service.rs wrapper ... already owns how that operation builds and maps its request; it should also own a sensible default RetryLayer/TimeoutLayer configuration for that specific operation, exposed so callers can use it as-is or override it via ServiceBuilder.

This matters more than usual here because the three operations have genuinely different latency profiles: the __typename-only status query should be fast and tightly bounded, while contractPreviewResult pulls full API + core schema documents and warrants a much longer timeout. A shared constant would be wrong for at least one of them. Where latency data is not available, CLAUDE.md asks for "a conservative default and say so in a comment" rather than leaving it unset.

Flagging transparently: no existing operation service.rs in this crate currently complies with this rule, so this is not a regression unique to this PR — but this is new code landing at exactly the path the rule targets, and per-operation defaults would also supply the per-attempt bound poll is missing above.