fix(sidecar): protocol 13 — a configuration write no longer waits for its reload (F9)
All checks were successful
PR Checks / rust-gates (pull_request) Successful in 3m48s
All checks were successful
PR Checks / rust-gates (pull_request) Successful in 3m48s
The plugin now answers config.write once the files are on disk and reports the reload as a config.outcome event, which this process files and feeds by its type like any other event. The reload window no longer has to fit inside REPLY_TIMEOUT, so CONFIG_RELOAD_WINDOW and the test that asserted the pairing are gone, and the 504 body stops pointing at a rollback. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E14m6SuuY6i1vASFeGDBeY
This commit is contained in:
@@ -102,9 +102,8 @@ pub async fn serve(addr: &str, state: AppState) -> anyhow::Result<()> {
|
||||
// an operator edited over SSH and the website then overwrote.
|
||||
.route("/config/files", get(config_files))
|
||||
.route("/config/file", get(config_file))
|
||||
// The only route on this sidecar that causes a WRITE on the game host, and the only one
|
||||
// whose reply can take most of the RPC budget: the plugin holds it open across a reload
|
||||
// and, at worst, across a rollback as well. See `CONFIG_RELOAD_WINDOW`.
|
||||
// The only route on this sidecar that causes a WRITE on the game host. It answers once the
|
||||
// files are on disk; the reload's outcome follows as a `config.outcome` event (protocol 13).
|
||||
.route("/config/write", post(config_write))
|
||||
// Protocol 8 (the module's PLAN.md §27): an event borrowing a value and giving it back.
|
||||
// Three correlated round trips, and the sidecar knows nothing about any of them — not
|
||||
@@ -694,19 +693,14 @@ async fn config_file(State(st): State<AppState>, Query(q): Query<ConfigPathQuery
|
||||
respond(st.rpc.call(&st.game, command, &req_id).await)
|
||||
}
|
||||
|
||||
/// The window the plugin gives a reload to announce itself, mirrored here from
|
||||
/// `ConfigReloadWindowSeconds` in `overlay/oxide/plugins/RunicGateway.cs`.
|
||||
///
|
||||
/// It is duplicated rather than negotiated because the two numbers are a *pairing*, like the
|
||||
/// protocol version: the plugin owns the behaviour and this end owns the budget it has to fit in.
|
||||
/// The test below is what keeps them honest — a worst-case write is two windows, and a
|
||||
/// [`REPLY_TIMEOUT`](crate::rpc::REPLY_TIMEOUT) that does not cover both would abandon the caller
|
||||
/// precisely when a rollback had just saved them, leaving the website to report a timeout over a
|
||||
/// server that is perfectly healthy.
|
||||
pub const CONFIG_RELOAD_WINDOW: Duration = Duration::from_secs(4);
|
||||
|
||||
/// Replace a set of configuration files and reload whatever owns them.
|
||||
///
|
||||
/// Since protocol 13 the reply does not wait for the reload. The plugin answers once the files are
|
||||
/// on disk (`pending: true`, a `writeId`, and the `ceilingMs` it will wait), and reports the reload
|
||||
/// as a `config.outcome` event when it settles. Protocol 12 held this RPC across the reload, which
|
||||
/// forced the plugin's window under [`REPLY_TIMEOUT`](crate::rpc::REPLY_TIMEOUT) — and a cold
|
||||
/// compile outlasted it and had a valid edit rolled back (the module's PLAN_FIXES.md F9).
|
||||
///
|
||||
/// Opaque body, exactly as `/permissions/sync`: `cmd` and `reqId` are written over whatever the
|
||||
/// caller sent, and nothing else about the object is read here. What the website sends is whole
|
||||
/// file *text* rather than a key and a value (D35), so there is nothing on this hop that could
|
||||
@@ -750,16 +744,15 @@ async fn config_write(State(st): State<AppState>, Json(body): Json<Value>) -> Re
|
||||
let result = st.rpc.call(&st.game, command, &req_id).await;
|
||||
|
||||
if matches!(result, Err(RpcError::Timeout)) {
|
||||
// A bare `504` on a route that writes reads as "did my change land, and is the plugin
|
||||
// still up?" — and unlike every other timeout on this sidecar, that question has a good
|
||||
// answer. The plugin writes a whole set or restores a whole set, never half of either, so
|
||||
// a re-read settles it; and a write that is still in flight is most likely inside the
|
||||
// rollback this window pays for.
|
||||
// A bare `504` on a route that writes reads as "did my change land?" — and unlike every
|
||||
// other timeout on this sidecar, that question has a good answer. The plugin writes a
|
||||
// whole set or none of it, and restores a whole set or none of it, so a re-read settles
|
||||
// it. Since protocol 13 the reply no longer waits for a reload, so this is a slow host or
|
||||
// a busy main thread, not a rollback in progress.
|
||||
return (
|
||||
StatusCode::GATEWAY_TIMEOUT,
|
||||
Json(json!({
|
||||
"error": "the plugin did not report within the write budget",
|
||||
"reloadWindowSeconds": CONFIG_RELOAD_WINDOW.as_secs(),
|
||||
"hint": "re-read the files: the plugin writes the whole set or restores it",
|
||||
})),
|
||||
)
|
||||
@@ -1037,25 +1030,6 @@ mod tests {
|
||||
assert_eq!(command["grants"], json!([]));
|
||||
}
|
||||
|
||||
/// A command larger than the game link's own line cap is refused here, where the caller learns
|
||||
/// why. Forwarded, it would be discarded by both ends without a word and present as a `504`.
|
||||
/// The pairing in `CONFIG_RELOAD_WINDOW`'s own words, asserted rather than trusted.
|
||||
///
|
||||
/// The worst path through one configuration write is two windows — wait for the edited
|
||||
/// plugin's reload, give up, restore the files, reload again — plus the write, the reads and
|
||||
/// a line each way. If that does not fit inside the RPC timeout, the caller is abandoned at
|
||||
/// exactly the moment the rollback saved it, and the website reports a timeout over a server
|
||||
/// that is healthy and has the old config back.
|
||||
#[test]
|
||||
fn a_rollback_fits_inside_the_rpc_budget() {
|
||||
let worst = CONFIG_RELOAD_WINDOW * 2;
|
||||
assert!(
|
||||
worst + Duration::from_secs(1) <= crate::rpc::REPLY_TIMEOUT,
|
||||
"two reload windows ({worst:?}) plus a second of slack must fit in {:?}",
|
||||
crate::rpc::REPLY_TIMEOUT
|
||||
);
|
||||
}
|
||||
|
||||
/// Protocol 8's routes cannot choose their own command or correlation id either, and they keep
|
||||
/// everything else the caller sent, unread.
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user