[ragnorc]

Summary

On a managed target, omnigraph cluster plan and cluster apply sent their submission under the fixed 10-second bound of every managed control request. --timeout only wrapped that request, so it could shorten the wait but never lengthen it. The managed control plane answers a plan submission only once it has captured the preview, and that can take longer than 10 seconds. The CLI then dropped the connection, reported transport_failed ... exceeded its 10-second deadline, and the capture did not finish.

The submission request now runs until the --timeout deadline (default 300 seconds, maximum 3600). The source lookup that plan makes without --rev, and delivery polling after apply, keep the 10-second bound.

Without --no-wait, reaching the deadline is still reported as wait_timeout (exit 5) with the idempotency key. Because the request's own bound now ends at that same deadline, a transport failure at or after the deadline is reported the same way, whichever timer fires first.

This is a stopgap. The lasting fix belongs in the managed control plane: accept the plan without holding the request open for the capture, and report the preview's progress through its status. Once that lands, this change stays harmless, because the submission simply returns sooner.

Tests

• New cli_cluster::managed_plan_and_apply_submissions_wait_until_their_local_deadline. Under --timeout 30, a plan reply held 10.5 s succeeds, and so does an apply --no-wait reply held 10.5 s. A --timeout 1 plan held 2.5 s exits 5 with wait_timeout, keeping its idempotency key. Each case sends exactly one request. On main the first case fails: exit 1, transport_failed ... exceeded its 10-second deadline. The test waits out a real 10-second bound, so it is #[ignore = "nightly: ..."] and enrolled in tests-slow-nightly.yml.
• cargo test -p omnigraph-cli --lib managed::: 100 passed.
• cargo test -p omnigraph-cli --test integration cli_cluster::managed_: 50 passed.
• cargo clippy -p omnigraph-cli --all-targets --no-deps -- -D warnings -W clippy::dbg_macro and cargo fmt --all --check on the pinned toolchain.

Docs: docs/user/cli/managed-lifecycle.md states the submission bound. The changelog note is changelog.d/managed-plan-apply-wait-for-timeout.fixed.md.

[ragnorc]

Recommendation: ready.

Why

On a managed target, cluster plan and cluster apply sent the submission under the fixed 10-second bound of an ordinary control call. --timeout wrapped that call, so it could only shorten the wait. The service answers a plan only after it captures the preview. A capture longer than 10 seconds made the CLI close the call. The CLI then reported transport_failed and exit 1.

What

The submission POST now runs until the --timeout deadline. The default is 300 seconds. The maximum is 3600 seconds. The source lookup and each delivery poll keep the 10-second bound. Without --no-wait, a hit on that deadline is wait_timeout and exit 5. The error keeps the idempotency key and says acceptance is unknown.

How

submit in crates/omnigraph-cli/src/managed/deployment.rs is the one owner of the plan POST and the apply POST. It passes the time left until the deadline to request_with_timeout. Reqwest 0.12.28 uses that per-request total timeout in place of the client total timeout (RequestBuilder::timeout overrides ClientBuilder::timeout). The client connect timeout stays 10 seconds. When the command waits, timeout_at uses the same deadline. If the call returns transport_failed at or after that deadline, submit reports wait_timeout.

The reqwest timer starts at send, after the session read. Its duration was taken before that read, so the timer ends after the deadline. The waiting path therefore stops on timeout_at. The clock check covers the same timer tick.

--no-wait uses the same POST bound. It does not use the clock check. A --no-wait POST that reaches the deadline stays transport_failed and exit 1. The JSON still carries the key. --no-wait --timeout 1 now bounds that POST at about 1 second. Before this change, --no-wait always used the 10-second client bound.

Impact

A plan or apply whose service answers after 10 seconds and before --timeout now returns that answer. The CLI still sends one POST. It does not retry. A closed call still means acceptance is unknown. The operator keeps the same idempotency key. The guide in docs/user/cli/managed-lifecycle.md states the new submission bound.

This is a client stopgap. The service still holds the HTTP call open until capture finishes. A proxy can still close that call. The lasting fix is for the managed control plane to accept the plan and report capture through status. This repository's server is the data plane. It does not serve /v1/clusters/*/plans. After that service change, this longer bound returns sooner. It does not block that change.

Optional findings

No blocking findings.

1. A transport failure at the deadline can be labeled wait_timeout. raw_bounded maps every send error and every body-read error to transport_failed. It drops reqwest's timeout flag. The new arm then maps that type to exit 5 when Instant::now() is at or after the deadline. A reset in that same tick exits 5. The same reset one tick earlier stays exit 1. Both results keep the key and say acceptance is unknown. The CLI does not send a second POST. Map only a timeout, and leave a reset as transport_failed. With --no-wait, the deadline stays exit 1. The new paragraph states exit 5 only for the waiting path. Name the --no-wait exit in that paragraph so a script does not treat exit 1 as a clean failure and mint a new key.

2. The nightly test guards the 10-second cap. It does not guard the relabel, a missed deadline above 10 seconds, the 10-second source lookup, or a --no-wait deadline. The third case uses --timeout 1. That case already passed on the merge base, because the old timeout_at returns wait_timeout under the 10-second cap. The first two cases are the regression.

Tradeoff and liability

Correctness first. --timeout is the local deadline the operator set. The hidden 10-second cap made that deadline false for the one call that does the work. Putting the deadline on submit fixes that class for both POSTs. Lookup and polls are separate calls and keep REQUEST_TIMEOUT.

Simplicity next. The patch adds a duration argument the client already had (raw_with_timeout) and one clock check. It does not add a queue, a stored plan, or a second publication path. The cost is two timers for one deadline, plus a classification that cannot see a timeout apart from any other transport error. Team calls already pass TEAM_TIMEOUT. Pull passes EXPORT_TIMEOUT. Durations are chosen at the call site today. Do not copy this clock check into the next slow call. If a second caller needs it, record "this was a timeout" on Failure inside raw_bounded, and let submit read that flag.

The idempotency key is printed before the POST. The error body keeps it. This CLI sends the POST once. A retry with the same key is a later command. The user guide says that replay returns the same reservation and that a changed body is refused. This repository documents that contract. It does not implement the control plane that must enforce it. If the service stores the key only when it sends the response, a client that stops at the deadline can leave a capture running, and a new key can create a second plan. Lifecycle commands save the key on disk before send. Plan and apply do not. That gap is older than this change. This patch does not make the CLI retry.

A connection may now stay open for up to 3600 seconds. The default is 300 seconds. Access JWTs are documented to last at most 15 minutes, and the bearer is fixed before send. The default deadline fits in that window. A 3600-second deadline does not. This review did not observe whether the service checks that token only at the start of the call.

Five more changes of this shape would leave five clock checks and five opinions about exit 1 versus exit 5. One helper that takes a deadline and returns a timeout distinct from a reset would keep the invariant in one place. submit can be that helper for plan and apply until the control plane accepts asynchronously. A new type is not needed for this one call.

Net effect: the change removes a client bound that contradicted --timeout. It adds a long-lived call and one coarse deadline label. It does not add a second source of truth, and it does not acknowledge a graph change before the service responds.

Validation

Reviewed head 49e6ab6dc43dfb4604e43591e39b6dd0d40ef2a2. Merge base with origin/main is 17557824d2788953d4049fd9c19b1430bff46f60. No other review or comment is on this head. No open pull request changes this client timeout. The change does not read or write Lance. Lance 13.0.0 and the open storage pull requests do not affect this bound. GQT cannot express a CLI process deadline or this HTTP fixture. cli_cluster is the existing owner. No .gqt case is required.

The regression test was copied onto the merge base, with the production fix absent. Temporary edits stayed in an isolated checkout and were removed. Nothing was pushed.

Command, from the merge base, full test as in the pull request:

CARGO_TARGET_DIR=/workspace/target cargo test -p omnigraph-cli --test integration cli_cluster::managed_plan_and_apply_submissions_wait_until_their_local_deadline -- --ignored --exact --nocapture --test-threads=1

Result: failed in 10.53s. The plan case with --timeout 30 exited 1. The body type was transport_failed. The detail was managed API request failed or exceeded its 10-second deadline. The idempotency key was slow-key and acceptance was unknown.

The same command with only the apply --no-wait --timeout 30 case: failed in 10.55s with the same 10-second transport_failed detail.

The same command with only the --timeout 1 case: passed in 2.53s. That case does not show a difference between the two commits.

Command, at the pull request head, after cargo clean -p omnigraph-cli so the test binary was built from that commit:

same cargo test command.

Result: passed in 23.58s. That duration matches a 10.5s plan reply, a 10.5s apply --no-wait reply, and a 2.5s reply under --timeout 1.

Limitations

The fixture is localhost TLS. It does not include a proxy timeout, a dropped connection at the deadline, or the managed control plane. The nightly workflow was not run here. tests-slow-nightly.yml is not a required pull-request check. The connect-timeout path was inspected in reqwest 0.12.28 and in Api::new. It was not timed. The control plane's idempotency store was not inspected, because it is not in this repository.

Inline comment on crates/omnigraph-cli/src/managed/deployment.rs line 208:

Optional. This arm maps every transport_failed at or after the deadline to wait_timeout (exit 5). raw_bounded uses that type for every send error and every body-read error, and it drops the reqwest timeout flag. A reset in this same tick exits 5. The same reset one tick earlier stays exit 1. Both results keep the idempotency key and say acceptance is unknown, so this arm does not send a second POST. Map only a timeout. Leave a reset as transport_failed.

The waiting path usually stops on timeout_at first. The reqwest total timeout starts at send, after the session read, so it ends after deadline. This clock check is the tie. --no-wait does not use this arm. A --no-wait POST that reaches the deadline stays transport_failed and exit 1.

Inline comment on docs/user/cli/managed-lifecycle.md line 130:

Optional. This paragraph states exit 5 only when the command waits. With --no-wait, the same deadline stays transport_failed and exit 1. The JSON still carries the key. Name that exit in this paragraph so a script does not treat exit 1 as a clean failure and mint a new key.

Inline comment on crates/omnigraph-cli/tests/integration/cli_cluster.rs line 2027:

Optional. This third case already passes on merge base 17557824. The old timeout_at returns wait_timeout when --timeout is 1 second, because that is under the 10-second client bound. I ran this case alone there and it passed in 2.53s. The first two cases are the regression. They fail on that base with exit 1 and exceeded its 10-second deadline. This test does not close the socket before the deadline, and it does not let --no-wait reach the deadline.

[ragnorc]

Recommendation: ready.

Why

The new commit b0950fa7 merges origin/main at 0e91b00e (pull request 1209, accepted-preview polling). That merge leaves the plan and apply code from 49e6ab6 unchanged. Conflict resolution rewrote the guide paragraph.

What

One local deadline covers the submission and the later poll. dispatch builds one Instant from --timeout. submit gives the time left to the POST. preview::finish and delivery wait then use that same instant. The phases run in sequence. The command stops at that instant. The total wait stays inside --timeout.

Pull request 1209 polls after the POST returns a preview id. A service that answers only after capture still needs this longer POST. A service that returns capturing at once uses the poll for the time that remains. The longer POST ends when the service answers. The poll uses the remainder of the same deadline.

How

submit in crates/omnigraph-cli/src/managed/deployment.rs remains the owner of both POSTs. Reqwest 0.12.28 applies RequestBuilder::timeout in place of the client total timeout. RequestConfig::fetch reads the request extension first. The client connect timeout stays 10 seconds. Source lookup and each poll still call api.request or api.raw, which pass REQUEST_TIMEOUT (10 seconds).

On the waiting path, timeout_at and the reqwest timer share the deadline. The reqwest timer starts at send, after the session read, so it ends after the deadline. timeout_at stops the command. The clock check covers the same timer tick. A transport_failed result at or after the deadline becomes wait_timeout (exit 5). Acceptance is unknown. The key stays in the body.

--no-wait uses the same POST bound. It skips the clock check. A POST that reaches the deadline stays transport_failed and exit 1.

A preview-poll deadline is a different exit 5. preview::finish returns the accepted preview and the key. The type is not wait_timeout. A transport error during a poll is logged, and the poll continues until the deadline. The POST is sent once.

Docs

The merged paragraph matches the bound. Submissions stay open until the --timeout deadline. Each source lookup and each poll keeps the 10-second HTTP bound.

Conflict resolution removed the sentence that a waiting submission deadline is exit 5 and that acceptance is unknown. The next paragraph is the poll resume from pull request 1209. It says to repeat plan with the same --rev and key, and that the original preview deadline does not change. That resume is true after the POST returns a preview. A POST that hits the deadline has no preview id. The JSON type is wait_timeout and submission.acceptance is unknown. Repeating plan with the same key is the safe action for both outcomes. The guide should name the two outcomes separately. With --no-wait, that POST deadline stays exit 1.

Optional findings

No blocking findings. Both earlier optional findings remain open.

1. The deadline arm still maps every transport_failed at or after the deadline to wait_timeout. raw_bounded uses that type for every send error and every body-read error, and it drops the reqwest timeout flag. A reset in that tick exits 5. The same reset one tick earlier stays exit 1. Both results keep the key and say acceptance is unknown. The CLI sends one POST. Map only a timeout. Leave a reset as transport_failed. The merged guide no longer says the waiting submission deadline is exit 5 with unknown acceptance. Name that outcome, and name --no-wait exit 1, apart from the accepted-preview resume.

2. The nightly test still guards the 10-second cap. It does not guard the relabel, a missed deadline above 10 seconds, the 10-second source lookup, or a --no-wait deadline. The third case uses --timeout 1. That case passes on merge base 0e91b00e in 2.53 seconds, because the old timeout_at already returns wait_timeout under the 10-second cap. The ready fixture includes native_plan, so preview::finish returns the POST body and does not poll. The test still exercises the submission path after the merge.

Tradeoff and liability

Correctness first. --timeout is the local deadline. The hidden 10-second cap made that deadline false for the POST. The same instant now bounds the POST and the poll from pull request 1209. Lookup and polls stay on REQUEST_TIMEOUT.

Simplicity next. The patch adds a duration the client already had, and one clock check. It adds no queue, no stored plan, and no second publication path. The cost is two timers for one POST deadline, and a label that cannot see a timeout apart from any other transport error. The poll already has its own rule for transport_failed: continue until the deadline. Keep the clock check on submit only.

The idempotency key is printed before the POST. A lost response means acceptance is unknown. This CLI sends the POST once. A later command with the same key is the resume. Pull request 1209 resumes a known preview by that same command. The two stories meet there. This repository documents the idempotent replay. The control plane that must enforce it is outside this repository.

Five more changes of this shape would leave five clock checks. One flag on Failure, set inside raw_bounded, would let submit read that the error was a timeout. submit can keep the check until the control plane accepts without holding the POST. A new type is not needed. The deadline Instant in dispatch is already the single owner of the budget.

Net effect: the change removes a client bound that contradicted --timeout. The poll from pull request 1209 starts only after the POST returns, so the longer POST bound remains the fix for a slow accept. The change adds a long-lived call and one coarse label. It adds no second source of truth. It does not acknowledge a graph change before the service responds.

Validation

Reviewed head b0950fa76fdd9c03b650c8dbfd9015c63f38cb11. Merge base with origin/main is 0e91b00e669f60f744b4952b54b3676d7744fe65. The earlier review is comment 6520894882 on head 49e6ab6dc43dfb4604e43591e39b6dd0d40ef2a2. No other review or comment is on this head. No other open pull request changes this client timeout. The diff does not read or write Lance. Lance 13.0.0 stays the pin in docs/dev/lance.md. GQT cannot express a CLI process deadline or this HTTP fixture. cli_cluster is the existing owner.

The regression test was copied onto the merge base. The production fix was absent. Temporary edits stayed in an isolated checkout and were removed. Nothing was pushed.

Command, merge base, full test:

CARGO_TARGET_DIR=/tmp/target-base cargo test -p omnigraph-cli --test integration cli_cluster::managed_plan_and_apply_submissions_wait_until_their_local_deadline -- --ignored --exact --nocapture --test-threads=1

Result: failed in 10.52s. The plan case with --timeout 30 exited 1. The body type was transport_failed. The detail was managed API request failed or exceeded its 10-second deadline. The idempotency key was slow-key and acceptance was unknown.

The same command with only the apply --no-wait --timeout 30 case: failed in 10.53s with the same 10-second transport_failed detail.

The same command with only the --timeout 1 case: passed in 2.53s. That case does not show a difference between the two commits.

Command, at this head, built from that commit:

same cargo test command, with CARGO_TARGET_DIR=/workspace/target.

Result: passed in 23.60s. That duration matches a 10.5s plan reply, a 10.5s apply --no-wait reply, and a 2.5s reply under --timeout 1. assert_complete requires one workflow request, so the ready body did not start a poll.

Also at this head:

CARGO_TARGET_DIR=/workspace/target cargo test -p omnigraph-cli --test integration cli_cluster::managed_ -- --test-threads=8

Result: 51 passed, 0 failed, 6 ignored, in 1.61s. That run includes the plan replay, the exact --no-wait submit, and managed_lost_submission_retains_key_without_effect_retry (a fast bad body stays exit 1).

GitHub Test Workspace on this head completed with success. That is CI evidence. The local runs are separate.

Limitations

The fixture is localhost TLS. It does not include a proxy timeout, a dropped connection at the deadline, or the managed control plane. The nightly workflow was not run here. tests-slow-nightly.yml is not a required pull-request check. The connect-timeout path was read in reqwest 0.12.28 and in Api::new. It was not timed.

Inline comment on crates/omnigraph-cli/src/managed/deployment.rs line 210:

Optional. Still open after the merge with pull request 1209. This arm maps every transport_failed at or after the deadline to wait_timeout (exit 5). raw_bounded uses that type for every send error and every body-read error, and it drops the reqwest timeout flag. A reset in this tick exits 5. The same reset one tick earlier stays exit 1. Both results keep the key and say acceptance is unknown. This arm sends no second POST. The preview poll treats transport_failed as a retry until the deadline. This POST does not. Map only a timeout. Leave a reset as transport_failed.

--no-wait skips this arm. A --no-wait POST that reaches the deadline stays transport_failed and exit 1.

Inline comment on docs/user/cli/managed-lifecycle.md line 134:

Optional. This paragraph states the submission bound, and that bound matches the code. Conflict resolution removed the older sentence that a waiting deadline is exit 5 and that acceptance is unknown. The next paragraph is the poll resume: repeat plan with the same --rev and key, because the original preview deadline does not change. That resume is true after the POST returns a preview. A POST that hits the deadline has no preview id. The JSON type is wait_timeout and submission.acceptance is unknown. Repeating plan with the same key is safe for both outcomes. Name them separately. With --no-wait, the same POST deadline stays transport_failed and exit 1.

Inline comment on crates/omnigraph-cli/tests/integration/cli_cluster.rs line 2035:

Optional. Still open. This third case passes on merge base 0e91b00e in 2.53s. The old timeout_at already returns wait_timeout when --timeout is 1 second, under the 10-second client bound. I ran this case alone there. The plan case with --timeout 30 fails on that base in 10.52s: exit 1, transport_failed, detail exceeded its 10-second deadline, key slow-key, acceptance unknown. The apply --no-wait --timeout 30 case fails there in 10.53s with the same detail. A ready body includes native_plan, so preview::finish returns this POST body and does not poll. The test still exercises the submission path. It does not close the socket before the deadline, and it does not let --no-wait reach the deadline.